Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Unmark Firefox as partial for::highlight CSS selector#28375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
caugner wants to merge2 commits intomain
base:main
Choose a base branch
Loading
fromhighlight-selector-in-firefox

Conversation

@caugner
Copy link
Contributor

Summary

Unmarks Firefox as partial for::highlight CSS Selector, and splits up the existing notes, adding bug links.

Test results and supporting details

I challenge the followingrequirement for partial implementation:

The browser's support has a demonstrable negative impact on web developers.

Related issues

Related:#28374

Also splits up the notes, adding bug links.
@caugnercaugner requested a review fromddbeckNovember 4, 2025 12:12
@github-actionsgithub-actionsbot added data:cssCompat data for CSS features. https://developer.mozilla.org/docs/Web/CSS size:s[PR only] 7-24 LoC changed labelsNov 4, 2025
@github-actions
Copy link
Contributor

github-actionsbot commentedNov 4, 2025
edited
Loading

Tip: Review these changesgrouped by change (recommended for most PRs), orgrouped by feature (for large PRs).

"edge":"mirror",
"firefox": {
"version_added":"140",
"partial_implementation":true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see the partial stay.

I think it's hard to make the case that lack of support here does not have developer impact. Most critically, there isan interop proposal with upvotes.

Moreover, I don't think this represents some subtle or implicit behavior. Boththe docs andthe spec explicitly call out that 1) this pseudo only works with specific properties and 2) these two properties are included.

When this was originally added to BCD, it was notedby the implementer that it was "not feature complete" who suggested that the release notes ought to say "partial support" (note the discussion here).

Copy link
Contributor

Choose a reason for hiding this comment

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

Being able to apply a text decoration to a custom highlight is one of the most important use cases for this API (that and background color). So removing the partial flag without having support for text-decoration seems premature to me.
That being said, I was told support for this property is in Firefox Beta. So perhaps we could remove the partial flag but also update the version to match whatever Beta is?

Copy link
Contributor

@captainbrossetcaptainbrossetNov 12, 2025
edited
Loading

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that the test linked in the second bug is also failing in Safari. So whilst I agree that text-decoration is important here and was sufficient grounds to mark the implementation as partial, it's unclear to me that the remaining bug is sufficient to block changing the status. Or, if it is we need to also mark the Safari implementation as partial and have much better clarity on what's actually needed to be considered a full implementation, both in this case specifically, and as a matter of process for other cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested a bit withthis demo my team and I made recently. If all you do is settext-decoration without doing anything else special, then it just works. However, if you start moving the line down withtext-underline-offset, then at some point the line gets clipped and no longer appears.

On the one hand, it makes sense to consider this partial, because otherwise developers might get bitten by this buggy behavior.
On the other hand, 99% of cases are probably just going to work.

In any case, whether we decide to retain the partial flag or not, I agree that Safari and Firefox should match. There's no reason to block Firefox's support of the feature because of a partial flag when Safari is affected by the same bug.

My personal opinion: remove the partial flag, and make Firefox's version_added be 146.

Comment on lines +24 to +25
"Cannot be used with `text-decoration`. See [bug 1987618](https://bugzil.la/1987618)",
"Cannot be used with `text-shadow`. See [bug 1845447](https://bugzil.la/1845447)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I do really like that this is two separate notes, with bugs for each. I do prefer it when notes are complete sentences, however. Something like:

Suggested change
"Cannot be used with `text-decoration`. See [bug 1987618](https://bugzil.la/1987618)",
"Cannot be used with `text-shadow`. See [bug 1845447](https://bugzil.la/1845447)"
"The pseudo-element cannot be used with `text-decoration`. See [bug 1987618](https://bugzil.la/1987618)",
"The pseudo-element cannot be used with `text-shadow`. See [bug 1845447](https://bugzil.la/1845447)"

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I disagree, because your suggestion makes the sentence 50% longer without adding any additional information not already included in the context.

Starting the notes with "Cannot be used" or similar makes it easier/faster for users to parse. In your suggestion, both notes share the first 38 characters, which is twice as much as "Cannot be used with".

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there are now three issues raised about the note here: reading speed, length, and completeness. Unfortunately, they're in competition with each other, but here's another try:

Suggested change
"Cannot be usedwith `text-decoration`. See [bug 1987618](https://bugzil.la/1987618)",
"Cannot be usedwith `text-shadow`. See [bug 1845447](https://bugzil.la/1845447)"
"The `text-decoration` property doesn't workwith `::highlight()`. See [bug 1987618](https://bugzil.la/1987618)",
"The `text-shadow` property doesn't workwith `::highlight()`. See [bug 1845447](https://bugzil.la/1845447)"

To improve reading speed and skimmability, we ought to move unique text to the head of the notes, to avoid visual repetition from the left.

That makes it harder to go short. We could trim it down to something like "text-shadow doesn't work" but that starts to make real compromises on meaning. (I ran into this somewhat often in the notes audit, where some notes were so terse I had to read the linked bug anyway.)

My new suggestion still solves the completeness problem. Mostly I suggest complete sentences because, while shorter notes look fine on MDN on desktop they look much worse on caniuse or MDN on mobile, where notes are not necessarily arranged horizontally with the subject of the note (or sometimes even on the same screen, in case of MDN on mobile).

(I think there are other good reasons for complete sentences, such as friendliness to English learners and professional pride, but I will not belabor the point further.)

@caugnercaugner added the meeting agendaIssues or pull requests in need of discussion in a project meeting. labelNov 7, 2025
@caugnercaugner marked this pull request as draftNovember 12, 2025 10:50
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ddbeckddbeckddbeck requested changes

+2 more reviewers

@jgrahamjgrahamjgraham left review comments

@captainbrossetcaptainbrossetcaptainbrosset left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

data:cssCompat data for CSS features. https://developer.mozilla.org/docs/Web/CSSmeeting agendaIssues or pull requests in need of discussion in a project meeting.size:s[PR only] 7-24 LoC changed

Projects

Status: Todo

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@caugner@ddbeck@jgraham@captainbrosset

[8]ページ先頭

©2009-2025 Movatter.jp