1367592 - stylo: Ensure we restyle the whole document when the root font-size changes.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

Similar to 1367589, a font-size change on the document element should cause a whole document restyle if it uses rem units.

I was thinking that some cascade optimizations may make us break this, but the fact that rem units may be used down the tree makes it unavoidable anyway.

I don't recall seeing code for doing this off-hand. Heycam, is it handled elsewhere in Gecko?

Assuming it's not, we'll need to fix this.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

ni? heycam for the question above (sorry, I'd look into it myself, but I need to run right now)

Comment 2

[Cameron McCormack (:heycam)]

In Gecko we handle root font-size changes here:

http://searchfox.org/mozilla-central/rev/35b37316149108d53a02fb8498e250ea8a22ab5d/layout/base/GeckoRestyleManager.cpp#123-143

So we check if we're restyling the root element, and if so, convert the current restyle process into a "rebuild all style data" one.  Effectively that will just add eRestyle_ForceDescendants to the restyle hint for the root, which will cause us to recascade the entire tree.  I'm just starting to look into handling the cascading stuff in bug 1367647.

Comment 3

[Cameron McCormack (:heycam)]

Servo has code here to update the root font size on the Device when we restyle the root element:

  http://searchfox.org/mozilla-central/rev/31eec6f59eb303adf3318058cb30bd6a883115a5/servo/components/style/properties/properties.mako.rs#2737-2740

but we still need something that adds the RECASCADE_DESCENDANTS flags to the restyle hint for children, when we detect the change.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

We could use the CascadeInfo stuff for that, I guess.

Comment 5

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=72b35d130a7658957e345baa8ca07bf896b2131b

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1367592 - Test.

Review commit: https://reviewboard.mozilla.org/r/145544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/145544/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8874078 [details]
Bug 1367592 - Test.

https://reviewboard.mozilla.org/r/145544/#review149482

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8874078 [details]
Bug 1367592 - Test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/145544/diff/1-2/

Comment 9

[Cameron McCormack (:heycam)]

https://github.com/servo/servo/pull/17151

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a570baba7458
Test. r=emilio

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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