Bug 149242 – HTMLSlotElement should render its assigned nodes

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ryosuke Niwa]

Support rendering the assigned nodes under HTMLSlotElement added in the bug 149241.

[Radar WebKit Bug Importer]

<rdar://problem/22750917>

[Antti Koivisto]

Created attachment 261686 [details]
patch

[Ryosuke Niwa]

Comment on attachment 261686 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261686&action=review

> Source/WebCore/dom/SlotAssignment.cpp:187
> +        auto defaultSlotEntry = m_slots.find(emptyAtom);

!? This would result in a hash lookup in each iteration.

> Source/WebCore/style/StyleResolveTree.cpp:473
> +static void attachSlot(HTMLSlotElement& slot, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition)

Attach sounds like we're attaching a renderer for a slot.
Maybe attachSlotContents would sound better?
attachSlotAssignee?

> Source/WebCore/style/StyleResolveTree.cpp:481
> +                continue;
> +            }
> +            if (is<Element>(*child))

Why don't we just do elseif?

> Source/WebCore/style/StyleResolveTree.cpp:586
> +                continue;
> +            }
> +            if (is<Element>(*child))

Why don't we just do "else if" instead?

> Source/WebCore/style/StyleResolveTree.cpp:616
> +#if ENABLE(SHADOW_DOM)
> +    else if (is<HTMLSlotElement>(current))
> +        detachSlot(downcast<HTMLSlotElement>(current), detachType);
> +#endif
> +    else if (ShadowRoot* shadowRoot = current.shadowRoot())
>          detachShadowRoot(*shadowRoot, detachType);

What happens when a slot element has its own shadow root?

> Source/WebCore/style/StyleResolveTree.cpp:857
> +                continue;
> +            }
> +            if (is<Element>(*child))

Ditto.

[Antti Koivisto]

(In reply to comment #3)
> Comment on attachment 261686 [details]
> patch
> 
> View in context:
> !? This would result in a hash lookup in each iteration.

This fixes a bug, the cached iterator becomes invalid after mutation.
 
> > Source/WebCore/style/StyleResolveTree.cpp:616
> > +#if ENABLE(SHADOW_DOM)
> > +    else if (is<HTMLSlotElement>(current))
> > +        detachSlot(downcast<HTMLSlotElement>(current), detachType);
> > +#endif
> > +    else if (ShadowRoot* shadowRoot = current.shadowRoot())
> >          detachShadowRoot(*shadowRoot, detachType);
> 
> What happens when a slot element has its own shadow root?

We ignore shadow root for slot element when creating the render tree so no need to detach it either. I assume the spec forbids it.

[Antti Koivisto]

https://trac.webkit.org/r190084