Closed Bug 1429846 Opened 2 years ago Closed 2 years ago

Fix invalidation for ::slotted.


(Core :: CSS Parsing and Computation, enhancement)

Not set



Tracking Status
firefox59 --- fixed


(Reporter: emilio, Assigned: emilio)




(1 file)

I overlooked something while implementing it.

I think the last bug remaining is handled nested slots in invalidate_slotted_children. But that's less common.

I've left a FIXME commit for that too in This patch is on top.
Filed for the Chromium issue, with a more detailed description, which I think it's what the bug is.
If it's not too hard, could you attach a patch that shows what the diff is from the state with the referenced commit entirely backed out, to the state with the partial backout, so I can more easily see which changes will remain?
Flags: needinfo?(emilio)
It doesn't apply cleanly on top of the other one, so I'm not sure how could I do that... But the only change remaining should be moving each_xbl_stylist to operate on CascadeData instead of Stylist (with the appropriate renaming), which I still want for Shadow DOM.
Flags: needinfo?(emilio)
Comment on attachment 8941900 [details]
Bug 1429846: Fix slotted invalidation.

::: commit-message-46008:16
(Diff revision 2)
> +Curiously, Blink fails the test as written, presumably because they don't flush
> +styles from getComputedStyle incorrectly (in their test they do via

s/don't // or s/incorrectly/correctly/

::: servo/components/style/
(Diff revision 2)
> +                            self.slotted_rules.get_or_insert_with(|| {
> +                                Box::new(Default::default())
> +                            })


would work too.

::: testing/web-platform/tests/css/css-scoping/slotted-invalidation.html:29
(Diff revision 2)
> +  var root = host.attachShadow({"mode":"open"});
> +  root.innerHTML = '<style>.outer ::slotted(#slotted) { background-color: red } .outer .inner::slotted(#slotted) { background-color: green }</style><div id="outer"><slot id="inner"></slot></div>';
> +
> +  assert_equals(window.getComputedStyle(slotted).backgroundColor, "rgba(0, 0, 0, 0)");
> +
> +  host.offsetTop;

Based on your comment, these flushes aren't needed, yes?  If so, please remove them.
Attachment #8941900 - Flags: review?(cam) → review+
Pushed by
Fix slotted invalidation. r=heycam
Pushed by
Skip the test in non-stylo on a CLOSED TREE. r=me
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.