Bug 172822 - We don't invalidate descendants when the matching status of a ::slotted selector changes.
Summary: We don't invalidate descendants when the matching status of a ::slotted selec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emilio Cobos Álvarez
URL:
Keywords:
Depends on:
Blocks: 172753
  Show dependency treegraph
 
Reported: 2017-06-01 10:11 PDT by Emilio Cobos Álvarez
Modified: 2017-06-02 03:18 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.60 KB, patch)
2017-06-01 10:57 PDT, Emilio Cobos Álvarez
no flags Details | Formatted Diff | Diff
Patch (3.70 KB, patch)
2017-06-01 11:12 PDT, Emilio Cobos Álvarez
no flags Details | Formatted Diff | Diff
Patch (4.15 KB, patch)
2017-06-01 14:00 PDT, Emilio Cobos Álvarez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (6.93 MB, application/zip)
2017-06-01 16:06 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Emilio Cobos Álvarez 2017-06-01 10:11:18 PDT
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.
Comment 1 Emilio Cobos Álvarez 2017-06-01 10:57:09 PDT
Created attachment 311727 [details]
Patch
Comment 2 Emilio Cobos Álvarez 2017-06-01 11:12:35 PDT
Created attachment 311730 [details]
Patch
Comment 3 Emilio Cobos Álvarez 2017-06-01 11:27:46 PDT
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?
Comment 4 Emilio Cobos Álvarez 2017-06-01 14:00:02 PDT
Created attachment 311757 [details]
Patch
Comment 5 Emilio Cobos Álvarez 2017-06-01 14:00:52 PDT
(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.
Comment 6 Build Bot 2017-06-01 16:06:47 PDT
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
Comment 7 Build Bot 2017-06-01 16:06:49 PDT
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
Comment 8 WebKit Commit Bot 2017-06-02 03:18:57 PDT
Comment on attachment 311757 [details]
Patch

Clearing flags on attachment: 311757

Committed r217708: <http://trac.webkit.org/changeset/217708>
Comment 9 WebKit Commit Bot 2017-06-02 03:18:59 PDT
All reviewed patches have been landed.  Closing bug.