Closed Bug 1068477 Opened 6 years ago Closed 6 years ago
When a property is dynamically enabled, allow its declarations in UA stylesheets to be honored without needing a restart
[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.)
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
(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.
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.
(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.
Happy to look at this when I return.
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
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.
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)
Attachment #8520428 - Attachment is patch: true
See Also: → 1097355
Comment on attachment 8520428 [details] [diff] [review] patch r=me
Attachment #8520428 - Flags: review?(bzbarsky) → review+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.