When a property is dynamically enabled, allow its declarations in UA stylesheets to be honored without needing a restart

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dholbert, Assigned: heycam)

Tracking

Trunk
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
[Spinning this off as suggested in bug 1067634.]

Right now, for pref-controlled CSS properties, we have a bit of a problem.

The good news is, properties can be dynamically controllable via prefs -- the pref lets you dynamically enable the propert without needing to restart. (And it's enabled for all subsequent stylesheet loads, IIRC)

There's one key exception to that, though: if the property is used in a UA stylesheet, then that usage will *not* be dynamically controlled by your pref enabling/disabling, because the UA stylesheet doesn't get re-parsed after you've flipped the pref -- not until you restart.

(This may or may not matter -- that depends on the UA-stylesheet rule in question.  But it can generally result in broken layout at the very least, until you restart. Anyway -- I'm filing this bug on figuring out a way to remove the need to restart.


Two thoughts on strategies to fix this (though I'm not sure how feasible either of these are with our current architecture):

 (1) maybe we can somehow mark all UA stylesheets as needing to be re-parsed for subsequent document loads, whenever a CSS-property-controlling pref is flipped.

 (2) we could (somehow) just treating CSS-enabling-prefs as always-enabled for UA stylesheets. (so it wouldn't matter whether the pref is set or not -- the UA stylesheet would honor it, and it'd influence the cascade and end up producing the nsStyleStruct contents that you'd expect, etc.  The property still wouldn't be exposed to content via getComputedStyle, though.)
(Reporter)

Updated

4 years ago
Summary: Re-evaluate UA stylesheets after a pref is enabled/disabled, to pick up now-valid declarations that involve those rules → When a property is dynamically enabled, allow its declarations in UA stylesheets to be honored without needing a restart
(Assignee)

Comment 1

4 years ago
(1) sounds like a much smaller change.  It might even be as simple as adding a method to nsLayoutStylesheetCache that parses/creates new sheets and calling it from our pref change observer.
(Assignee)

Comment 2

4 years ago
I think it's probably fine for pages to get odd layout if you flip a pref and don't reload the document, too.  If it's not fine, then this problem really applies to author style sheets too, and you might want to re-parse them (whether they use @supports or not).
(2) is not great, because some of those rules actually depend on other parts of the system having the pref set too.

(1) should be fairly doable just by blowing away the stylesheet cache and reparsing stuff.
Duplicate of this bug: 1090744
(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #1)
> (1) sounds like a much smaller change.  It might even be as simple as adding
> a method to nsLayoutStylesheetCache that parses/creates new sheets and
> calling it from our pref change observer.

Are you interested in fixing this? It blocks some of my tests for ruby.
Blocks: 1084183
Flags: needinfo?(cam)
(Assignee)

Comment 6

4 years ago
Happy to look at this when I return.
(Assignee)

Comment 7

4 years ago
I got carried away for a bit over-engineering a solution to this (to avoid having to write code in multiple places whenever we want a sheet to be reloaded for a given pref change), but it was too much work for little gain.  So let's do something simple -- have nsLayoutStylesheetCache::EnsureGlobal register a (single) pref callback for each pref we care about, and have the callback blow away all of the sheets that depend on a pref.

One thing, though.  The ruby pref is used in ua.css, which is one of the sheets we load eagerly.  Is there any reason to keep this and not make all of the sheets be loaded lazily (or at least, make ua.css load lazily)?
Status: NEW → ASSIGNED
Flags: needinfo?(bzbarsky)
(Assignee)

Updated

4 years ago
Flags: needinfo?(cam)
We load ua.css eagerly because we know we'll need it, so might as well start early.  But I guess it's also a sync load...

You could try loading it lazily and seeing how startup performance is affected.
Flags: needinfo?(bzbarsky)
IIRC, this also causes incorrect results on reftests with bug 1052924.
Blocks: 1052924
(Assignee)

Comment 10

4 years ago
Created attachment 8520428 [details] [diff] [review]
patch

Here's the first half, which adds some hopefully obvious places to register for prefs and invalidate the cached sheets.  I'll put a second patch up for the ruby pref and ua.css in a separate bug.
Assignee: nobody → cam
Attachment #8520428 - Flags: review?(bzbarsky)
(Assignee)

Updated

4 years ago
Blocks: 1096808
(Assignee)

Updated

4 years ago
Attachment #8520428 - Attachment is patch: true
(Reporter)

Updated

4 years ago
See Also: → bug 1097355
No longer blocks: 1096639
Comment on attachment 8520428 [details] [diff] [review]
patch

r=me
Attachment #8520428 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/ba794ec8a0f0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.