Bug 172822 – We don't invalidate descendants when the matching status of a ::slotted selector changes.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Emilio Cobos Álvarez]

Consider the following test-case, straight from fast/shadow-dom/css-scoping-slot-with-id.html:

<my-host id="t4"><div>text</div></my-host>
<script>
var host = document.querySelector('#t4');
var shadow = host.attachShadow({ mode: 'open' });
shadow.innerHTML = `
  <style>
    #myslot::slotted(*) {
      background-color: green; width: 100%; height: 100%;
    }
  </style>
  <slot id="notmyslot" style="color:green"></slot>
`;
var slot = shadow.querySelector("#notmyslot");
slot.offsetWidth;
slot.setAttribute("id", "myslot");
</script>

This test-case is failing with my patch in bug 172753 because of the following:

We see that the <slot> element may change style and schedule it for a recalc, _but_ we don't invalidate the subtree, which may change because of the ::slotted pseudo-element, so when we see that the style hasn't changed for it we just don't recalc the inner <div>'s style.

This is wallpapered because right now we always force an Inherit change for display: contents elements, which ends up working in this test case.

[Emilio Cobos Álvarez]

Created attachment 311727 [details]
Patch

[Emilio Cobos Álvarez]

Created attachment 311730 [details]
Patch

[Emilio Cobos Álvarez]

Comment on attachment 311730 [details]
Patch

Gah, this is not going to be enough... I verified that the bug is fixed if I use is<HTMLSlotElement>, and assumed that shadowRoot() would return non-null for an attached slot (which isn't true).

But to properly handle descendant combinators I need to check if an element is a slot, or an ancestor of a slot...

Is (shadowRoot || is<HTMLSlotElement>(element)) the correct check for that?

[Emilio Cobos Álvarez]

Created attachment 311757 [details]
Patch

[Emilio Cobos Álvarez]

(In reply to Emilio Cobos Álvarez from comment #3)
> Comment on attachment 311730 [details]
> Patch
> 
> Gah, this is not going to be enough... I verified that the bug is fixed if I
> use is<HTMLSlotElement>, and assumed that shadowRoot() would return non-null
> for an attached slot (which isn't true).
> 
> But to properly handle descendant combinators I need to check if an element
> is a slot, or an ancestor of a slot...
> 
> Is (shadowRoot || is<HTMLSlotElement>(element)) the correct check for that?

Of course, descendant combinators are handled already correctly, so is<HTMLSlotElement> should be enough. I just updated the patch.

[Build Bot]

Comment on attachment 311757 [details]
Patch

Attachment 311757 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3856001

New failing tests:
compositing/masks/compositing-clip-path-change-no-repaint.html

[Build Bot]

Created attachment 311772 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[WebKit Commit Bot]

Comment on attachment 311757 [details]
Patch

Clearing flags on attachment: 311757

Committed r217708: <http://trac.webkit.org/changeset/217708>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.