canParticipateInFlatTree concept does not appear to make sense and LayoutTreeTraversal is confused about slots |
|||||||
Issue description
<slot> and active insertion points returns false from canParticipateInFlatTree(), but those elements can have a style, especially in this new display: contents world.
Usage of canParticipateInFlatTree() has created a bunch of bugs:
- Document::styleForElementIgnoringPendingStylesheets does:
if (!element->canParticipateInFlatTree())
return ensureStyleResolver().styleForElement(element, nullptr);
This means anyone doing styleForElementIgnoringPendingStylesheets on a <slot> is getting a totally wrong answer for the slot's style, while a div with display: contents does correctly get a style.
- Document::needsLayoutTreeUpdateForNode does:
if (!node.canParticipateInFlatTree())
return false;
This means anyone using Document::updateStyleAndLayoutTreeForNode gets the wrong answer for slots.
In fact the only reason some of the code appears to work today is that getComputedStyle() always unconditionally forces a layout when an element is in a shadow tree (which is wrong).
<slot> and all other display: contents things need to be treated the same by the style system and we need to remove all usage of canParticipateInFlatTree() inside it if we want these nodes to have styles.
ex. of a test that only passes because the system forces a recalc today:
fast/alignment/parse-alignment-of-root-elements.html
This styles a <slot> in a shadow tree and uses getComputedStyle() on it, which only works because of the bug where we force a layout.
This also seems wrong because I'm pretty sure <slot> with display: block should generate a box, and the only reason it doesn't is because of display: contents.
Today the LayoutTreeTraversal skips all slots, but does return display: contents things for parent(), but not for layoutParent(). We inherit the style from parent, but not from layoutParent(), which we only use for checks in StyleAdjuster. This doesn't work though, because <slot>'s are just display: contents things in the UA sheet.
LayoutTreeTraversal::parent() must return insertion points to get the inheritance chain correct.
,
Mar 23 2017
Yeah, once "display:contents" is supported, we should update FlatTreeTraversal so that it does include slots. As of now, FlatTreeTraversal is skipping slots intentionally because "display:contents" is not supported. We should align both things so that it does not have inconsistency.
,
Mar 23 2017
This is broken already for getComputedStyle() on any insertion point, it just works because of a bad hack. That needs fixing anyway, ignoring anything about display: contents.
,
Mar 23 2017
Do you have a bug link for that? getComputedStyle() should use FlatTreeTraversal::parent() or something, instead of Node#parent().
,
Mar 23 2017
Ah, I have found that: https://bugs.chromium.org/p/chromium/issues/detail?id=682869
,
Mar 23 2017
No, that's an unrelated bug, the bug I'm talking about is this one. Document::needsLayoutTreeUpdateForNode returns the wrong value for <slot> because it checks canParticipateInFlatTree and uses FlatTreeTraversal::parent, and we use that inside getComputedStyle by way of updateStyleAndLayoutTreeForNode.
,
Mar 23 2017
Thank you. It looks I misunderstood the point. Let me take a look at code around.
,
Mar 23 2017
,
Mar 23 2017
,
Apr 18 2017
It would be better to work on this after bug 660265 is resolved.
,
Dec 4 2017
,
Mar 5 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by esprehn@chromium.org
, Mar 23 2017