Closed Bug 1394233 Opened 4 years ago Closed 4 years ago

stylo: [non-e10s] Certain rule in userContent.css is not applied

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: arai, Assigned: emilio)

References

Details

(Keywords: regression)

Attachments

(2 files)

Steps to reproduce:
  1. run Nightly 57.0a1 (2017-08-27) (64-bit) with clean profile on macOS
  2. disable e10s
  3. restart Nightly
  4. create chrome directory in profile
  5. create userContent.css inside chrome directory, with the following content

.ProfileTweet-actionList {
    display: none !important;
}

  6. restart Nightly
  7. open https://twitter.com/FirefoxNightly
  8. reload the page (might not be necessary)

Actual result:
  the style is not applied and buttons under tweet (reply, retweet, favorite...) are shown

Expected result:
  the style is applied and those buttons are hidden.
  (see attached screenshot for the difference)


Some experiments and results:
  1. the following rule works, even when it's written after the above rule

body {
  border: 1px solid red !important;
}

  2. happens only on non-e10s.  both rules work on e10s

  3. regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c4eec1e81c108f8c97a4b228a7f27f26f925ffeb&tochange=98a6294158926166c18e57aec1f1adcff4f5e8a2
> Some experiments and results:
  4. happens only with Stylo enabled.
Summary: [non-e10s] Certain rule in userContent.css is not applied → [stylo + non-e10s] Certain rule in userContent.css is not applied
I have no idea how bug 1355721 can cause this :/

And btw, on stylo, the inspector doesn't seem to display any rule on the element... which is probably a different bug, though.
It's weird. It happens when the class contains any uppercase letter. That says, if you have a rule `.x { display: none }` in user sheet, and `<div class="x">`, it would work correctly. However, if you have a rule `.X { display: none }`, it would not be applied on `<div class="X">`.
P5 because this bug only affects non-e10s and userContent.css, not web content.
Priority: -- → P5
Summary: [stylo + non-e10s] Certain rule in userContent.css is not applied → stylo: [non-e10s] Certain rule in userContent.css is not applied
This is very strange indeed. I changed the uppercase-lowercase handling but just inside @font-feature-values rule. I don't think that can cause a bug like that. I'll investigate further.
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc0acc00d769
Reftest to ensure that uppercase class names work. r=test-only
Can anyone still reproduce the issue? I couldn't.
Keywords: leave-open
yes, happens on 57.0a1 (2017-09-15) (64-bit) on macOS, with the steps in comment #0.
I could not reproduce it on Windows 10, with the same STR.
macOS try run could not reproduce it, either (but failing on Linux x64 QuantumRender debug):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e0c0c0bdc29109178ba58279369e76f7bf7ea73

Can you reproducible it with a fresh profile?
the steps in comment #0 is based on fresh profile.
(of course it changes to non-e10s in step 2)

also, the issue happens with the html file and CSS rule in https://hg.mozilla.org/integration/mozilla-inbound/rev/cc0acc00d769
the paragraph is have a red background on non-e10s+stylo configuration.
Backed out for frequently failing own test usercss-uppercase.html on Windows 7 debug:

https://hg.mozilla.org/mozilla-central/rev/80494b13f4928660229d6c39d409cc4094a6b35a

See bug 1400610 for the failures.
Flags: needinfo?(VYV03354)
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5d8edcea43b
Reftest to ensure that uppercase class names work. r=test-only
Blocks: 1424878
Comment on attachment 8949780 [details]
Bug 1394233: Quirks mode changes can happen after a flush.

https://reviewboard.mozilla.org/r/219084/#review224992

So it's about selector_map which uses ascii lowercase atoms as key, okay.

Hope this wouldn't slow down things... It feels like this is going to put penalty on documents using standard mode in that case, because html document is default to quirks mode :/

::: servo/ports/geckolib/glue.rs:2449
(Diff revision 1)
> -    let quirks_mode = unsafe {
> -        (*data.stylist.device().pres_context().mDocument.raw::<nsIDocument>())
> -            .mCompatMode
> +    let doc =
> +        &*data.stylist.device().pres_context().mDocument.raw::<nsIDocument>();
> +    data.stylist.set_quirks_mode(QuirksMode::from(quirks_mode));

How is this code compiled at all? I don't see where the `quirks_mode` variable is from.
Attachment #8949780 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8949780 [details]
Bug 1394233: Quirks mode changes can happen after a flush.

https://reviewboard.mozilla.org/r/219084/#review224992

Yeah, that's right, but it only affects user sheets I think. UA sheets are cached, and document sheets are still not there when this happens.

Anyway I do want to profile which kind of style flushes really happen during initialization and try to avoid them.

> How is this code compiled at all? I don't see where the `quirks_mode` variable is from.

Whoops, pushed it too fast, then forgot to push the fixup.
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/80dbdb06f915
Quirks mode changes can happen after a flush. r=xidorn
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
I think this can ride the trains.
Assignee: nobody → emilio
Keywords: leave-open
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.