Skip to content

fix(highlight): adopt highlight styles into the host's tree scope - #2315

Open
rkaraivanov wants to merge 1 commit into
masterfrom
rkaraivanov/highlight-fix-firefox-scope
Open

fix(highlight): adopt highlight styles into the host's tree scope#2315
rkaraivanov wants to merge 1 commit into
masterfrom
rkaraivanov/highlight-fix-firefox-scope

Conversation

@rkaraivanov

Copy link
Copy Markdown
Member

Description

Matches are painted again in Firefox. The ::highlight() rules now live in the tree scope that owns the text they style, instead of in the component's own shadow root.

::highlight() rules are tree-scoped - they only reach text nodes belonging to the same tree scope as the stylesheet. The service adopted its sheet into renderRoot, but igc-highlight renders nothing except a <slot>, so the text it highlights is light DOM sitting in the host's scope. The two never matched up. Earlier Firefox versions resolved such rules across scopes, as Chromium still does, so the mismatch went unnoticed; once Firefox began enforcing the scoping, nothing was painted at all. Matching itself was never affected - size and the Range objects were correct throughout, only the styles failed to apply.

The sheet is now adopted by getRootNode(), which is the document or an outer shadow root depending on where the component sits. attachStylesheet stays idempotent and tracks where the sheet actually landed, so a host that moves between scopes drops it from the old root before adopting into the new one, and a host that is detached - or built inside a <template>, whose fragment has no adoptedStyleSheets - resolves to no root and is skipped.

hostConnected attaches the sheet directly rather than waiting on the theming controller's themeChange callback. That callback is still needed, since adoptStyles replaces adoptedStyleSheets wholesale on every theme change, but it is no longer the only path that gets the sheet installed.

This also settles a Chromium bug that predates the Firefox change: an igc-highlight nested inside another component's shadow root matched the cross-scope rule but resolved --background / --foreground against the wrong scope, painting the theme fallback instead of the configured colors.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

Covered by three regression tests over scope placement, re-targeting on move, and cleanup on disconnect.

Checklist

  • My code follows the project's coding standards
  • I have tested my changes locally
  • I have updated documentation if needed

Matches are painted again in Firefox. The `::highlight()` rules now live in
the tree scope that owns the text they style, instead of in the component's
own shadow root.

`::highlight()` rules are tree-scoped - they only reach text nodes belonging
to the same tree scope as the stylesheet. The service adopted its sheet into
`renderRoot`, but `igc-highlight` renders nothing except a `<slot>`, so the
text it highlights is light DOM sitting in the host's scope. The two never
matched up. Earlier Firefox versions resolved such rules across scopes, as
Chromium still does, so the mismatch went unnoticed; once Firefox began
enforcing the scoping, nothing was painted at all. Matching itself was never
affected - `size` and the `Range` objects were correct throughout, only the
styles failed to apply.

The sheet is now adopted by `getRootNode()`, which is the document or an
outer shadow root depending on where the component sits. `attachStylesheet`
stays idempotent and tracks where the sheet actually landed, so a host that
moves between scopes drops it from the old root before adopting into the
new one, and a host that is detached - or built inside a `<template>`, whose
fragment has no `adoptedStyleSheets` - resolves to no root and is skipped.

`hostConnected` attaches the sheet directly rather than waiting on the
theming controller's `themeChange` callback. That callback is still needed,
since `adoptStyles` replaces `adoptedStyleSheets` wholesale on every theme
change, but it is no longer the only path that gets the sheet installed.

This also settles a Chromium bug that predates the Firefox change: an
`igc-highlight` nested inside another component's shadow root matched the
cross-scope rule but resolved `--background` / `--foreground` against the
wrong scope, painting the theme fallback instead of the configured colors.

Covered by three regression tests over scope placement, re-targeting on move,
and cleanup on disconnect.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes igc-highlight rendering in browsers that enforce ::highlight() tree scoping (notably recent Firefox) by adopting the constructed ::highlight() stylesheet into the same tree scope as the highlighted text nodes (document or an outer shadow root), rather than into the component’s own shadow root (which only contains a <slot>).

Changes:

  • Resolve the correct “style root” via getRootNode() and adopt/remove the constructed highlight sheet in that root, tracking the currently attached root to support re-targeting.
  • Attach the stylesheet on hostConnected (in addition to the existing theming themeChange path) so styles are installed immediately on connect.
  • Add regression tests covering correct scope placement, re-targeting on move between scopes, and cleanup on disconnect.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/components/highlight/service.ts Moves ::highlight() stylesheet adoption from renderRoot to the host’s tree scope root and tracks/removes/re-attaches across scope changes.
src/components/highlight/highlight.spec.ts Adds regression tests validating stylesheet adoption scope, re-targeting after moving into another shadow root, and removal on disconnect.
CHANGELOG.md Documents the Firefox regression cause and the fix, plus the related Chromium custom-property resolution improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants