1384691 - stylo: There's another caller of HasAuthorSpecifiedRules that doesn't go through the pres context (text-shadow in selection may behave differently).

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

This is in [1].

It's somewhat annoying because it gives us a pseudo-element style... It's presumably not terrible to implement.

That being said, stylo doesn't fail any text-shadow-selected-x tests right now...

It's only used to set mHasSelectionShadow. But there are no text-shadow UA rules, so that will always return true iff mTextShadow is non-null, I think, except for user sheets...

Anyway, need to look into it more in detail.

[1]: http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/layout/generic/nsTextFrame.cpp#4111

Comment 1

[Matt Brubeck (:mbrubeck)]

Somewhat related to bug 1363088 (which is a bug in both Gecko and Stylo).

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I'm looking into whether we can actually just remove that call as well as mHasSelectionShadow.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1384691 - Unconditionally set mHasSelectionShadow when -moz-selection pseudo element is used.

The HasAuthorSpecifiedRules check was from the initial implementation
of text-shadow support on ::-moz-selection in bug721750. There doesn't
seem to be anything mentioning why this check is necessary.

Also given text-shadow is inherited by default, it doesn't seems this
change would lead to any difference.

Review commit: https://reviewboard.mozilla.org/r/179172/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179172/

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

try looks good (as expected): https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ddde5d1b779082c7703e595ff290233ac614f3b

Comment 5

[Jonathan Kew [:jfkthame]]

This is probably OK - at least, so far I don't see why it shouldn't be, though I'd like to look at it a bit more and try to understand what the code was attempting to do. But in looking at text selection examples, I'm seeing an alarming difference between non-stylo and stylo behavior, at least on macOS. (I mention this because selection highlighting differs significantly between platforms, so it's possible this is a Mac-only regression.)

Filed bug 1401317 to look into that.

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8907476 [details]
Bug 1384691 - Unconditionally set mHasSelectionShadow when -moz-selection pseudo element is used.

https://reviewboard.mozilla.org/r/179172/#review192866

Yeah, I think it should be fine to do this. Sorry for the delay here.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5909ec7d5d53
Unconditionally set mHasSelectionShadow when -moz-selection pseudo element is used. r=jfkthame

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5909ec7d5d53

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Does this need uplift to Beta or can it ride the 58 train?

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

I don't think it's important to uplift it to Beta. Let's just have it ride the 58 train.