Bug 99750 – [Shadow]: Fallback content should also be reprojected

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Dimitri Glazkov (Google)]

See https://www.w3.org/Bugs/Public/show_bug.cgi?id=19436

[Shinya Kawanaka]

I'll take a look.
I think it is working in the current implementation, but let's have a test to confirm it.

[Shinya Kawanaka]

Ah... It hits ASSERT!

[Shinya Kawanaka]

Even after fixing ASSERT, fallback content is not rendered.
Maybe we have to fix ParentWalker.

[Shinya Kawanaka]

- host
   - content
        - div (fallback)

We have assumed that only direct children of the host can be distributed, but it's wrong!

[Shinya Kawanaka]

I'm now working for this.

[Shinya Kawanaka]

Created attachment 169869 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 169869 [details]
Patch

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

> Source/WebCore/html/shadow/HTMLContentElement.h:65
> +inline HTMLContentElement* toHTMLContentElement(Node* node)

Is this used anywhere?

> Source/WebCore/html/shadow/InsertionPoint.h:123
> +inline Node* parentNodeForDistribution(const Node* node)

Why is this hanging in the InsertionPoint? And why are the two methods? Can't this just be an inline static helper inside of the ComposedShadowTreeWalker? It seems like you are prematurely building an API when there's only one consumer of this function.

[Shinya Kawanaka]

Comment on attachment 169869 [details]
Patch

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

>> Source/WebCore/html/shadow/InsertionPoint.h:123
>> +inline Node* parentNodeForDistribution(const Node* node)
> 
> Why is this hanging in the InsertionPoint? And why are the two methods? Can't this just be an inline static helper inside of the ComposedShadowTreeWalker? It seems like you are prematurely building an API when there's only one consumer of this function.

Actually we also need this in StyleResolver. We should have only one, though.

[Shinya Kawanaka]

Created attachment 170043 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 170043 [details]
Patch

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

> Source/WebCore/html/shadow/InsertionPoint.h:123
> +static inline Element* parentElementForDistribution(const Node* node)

I don't know if "static" makes any sense in this context.

[Shinya Kawanaka]

Created attachment 170049 [details]
Patch for landing

[Shinya Kawanaka]

(In reply to comment #10)
> (From update of attachment 170043 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170043&action=review
> 
> > Source/WebCore/html/shadow/InsertionPoint.h:123
> > +static inline Element* parentElementForDistribution(const Node* node)
> 
> I don't know if "static" makes any sense in this context.

It's a mistake! Removed.

[WebKit Review Bot]

Comment on attachment 170049 [details]
Patch for landing

Clearing flags on attachment: 170049

Committed r132174: <http://trac.webkit.org/changeset/132174>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.