1324341 - stylo: redo the cascade when CSSOM methods modify style sheets

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Currently, if we modify a style sheet with a CSSOM method like insertRule, we won't correctly restyle the document.  We need to tell the Stylist that its style sheets' contents have changed.

This causes layout/style/crashtests/645951-1.html (which is really a reftest) to fail.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Add FFI function to make the Stylist realize we have modified style sheets.

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

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Bug 1324341 - Part 1: Convert eRestyle_Subtree into (eRestyle_Self | eRestyle_SomeDescendants), so Servo will actually restyle descendants.

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

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Bug 1324341 - Part 2: Redo the cascade when CSSOM methods modify style sheets.

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

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8819740 [details]
Bug 1324341 - Part 2: Redo the cascade when CSSOM methods modify style sheets.

https://reviewboard.mozilla.org/r/99390/#review99702

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8819738 [details]
Add FFI function to make the Stylist realize we have modified style sheets.

https://reviewboard.mozilla.org/r/99386/#review99704

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8819739 [details]
Bug 1324341 - Part 1: Convert eRestyle_Subtree into (eRestyle_Self | eRestyle_SomeDescendants), so Servo will actually restyle descendants.

https://reviewboard.mozilla.org/r/99388/#review99706

I admit I'm not very sure about this, but I guess it is fine for now.

::: layout/base/ServoRestyleManager.cpp:69
(Diff revision 1)
> +  // RESTYLE_SELF | RESTYLE_DESCENDANTS, since this is a commonly
> +  // posted restyle hint that doesn't yet align with RestyleHint's
> +  // bits.
> +  if (aRestyleHint & eRestyle_Subtree) {
> +    aRestyleHint &= ~eRestyle_Subtree;
> +    aRestyleHint |= (eRestyle_Self | eRestyle_SomeDescendants);

The parens are not necessary here.

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8819739 [details]
Bug 1324341 - Part 1: Convert eRestyle_Subtree into (eRestyle_Self | eRestyle_SomeDescendants), so Servo will actually restyle descendants.

https://reviewboard.mozilla.org/r/99388/#review99706

Yeah, we need to expand our support for restyle hints on the Servo side so that eventually we don't need to do much of this kind of thing.

Comment 8

[Cameron McCormack (:heycam)]

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfc182766682
Part 1: Convert eRestyle_Subtree into (eRestyle_Self | eRestyle_SomeDescendants), so Servo will actually restyle descendants. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eee3d9d3ac1
Part 2: Redo the cascade when CSSOM methods modify style sheets. r=xidorn

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bfc182766682
https://hg.mozilla.org/mozilla-central/rev/1eee3d9d3ac1