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

RESOLVED FIXED in Firefox 58

Status

()

P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: emilio, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox55 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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
Somewhat related to bug 1363088 (which is a bug in both Gecko and Stylo).
Depends on: 1363088
Priority: -- → P3
(Assignee)

Comment 2

a year ago
I'm looking into whether we can actually just remove that call as well as mHasSelectionShadow.
Assignee: nobody → xidorn+moz
Comment hidden (mozreview-request)
status-firefox55: --- → wontfix
status-firefox56: --- → wontfix
status-firefox57: --- → affected
status-firefox-esr52: --- → wontfix
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

a year ago
mozreview-review
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.
Attachment #8907476 - Flags: review?(jfkthame) → review+

Comment 7

a year ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5909ec7d5d53
Unconditionally set mHasSelectionShadow when -moz-selection pseudo element is used. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/5909ec7d5d53
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
(Assignee)

Updated

a year ago
Blocks: 1363088
No longer depends on: 1363088
Does this need uplift to Beta or can it ride the 58 train?
Flags: needinfo?(xidorn+moz)
(Assignee)

Comment 10

a year ago
I don't think it's important to uplift it to Beta. Let's just have it ride the 58 train.
Flags: needinfo?(xidorn+moz)
status-firefox57: affected → wontfix
You need to log in before you can comment on or make changes to this bug.