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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

[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.)

Comment 1

[Cameron McCormack (:heycam)]

(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.

Comment 2

[Cameron McCormack (:heycam)]

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).

Comment 3

[Boris Zbarsky [:bzbarsky]]

(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.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

(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.

Comment 6

[Cameron McCormack (:heycam)]

Happy to look at this when I return.

Comment 7

[Cameron McCormack (:heycam)]

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)?

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

IIRC, this also causes incorrect results on reftests with bug 1052924.

Comment 10

[Cameron McCormack (:heycam)]

Attachment: 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.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8520428 [details] [diff] [review]
patch

r=me

Comment 12

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba794ec8a0f0

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ba794ec8a0f0