Bug 99750 - [Shadow]: Fallback content should also be reprojected
Summary: [Shadow]: Fallback content should also be reprojected
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shinya Kawanaka
URL:
Keywords:
Depends on: 99815
Blocks: 96986
  Show dependency treegraph
 
Reported: 2012-10-18 13:23 PDT by Dimitri Glazkov (Google)
Modified: 2012-10-22 20:09 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.21 KB, patch)
2012-10-22 03:30 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (11.28 KB, patch)
2012-10-22 19:21 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch for landing (11.28 KB, patch)
2012-10-22 19:43 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2012-10-18 13:23:37 PDT
See https://www.w3.org/Bugs/Public/show_bug.cgi?id=19436
Comment 1 Shinya Kawanaka 2012-10-18 20:38:38 PDT
I'll take a look.
I think it is working in the current implementation, but let's have a test to confirm it.
Comment 2 Shinya Kawanaka 2012-10-18 22:42:22 PDT
Ah... It hits ASSERT!
Comment 3 Shinya Kawanaka 2012-10-18 23:28:04 PDT
Even after fixing ASSERT, fallback content is not rendered.
Maybe we have to fix ParentWalker.
Comment 4 Shinya Kawanaka 2012-10-19 01:00:36 PDT
- host
   - content
        - div (fallback)

We have assumed that only direct children of the host can be distributed, but it's wrong!
Comment 5 Shinya Kawanaka 2012-10-19 01:28:25 PDT
I'm now working for this.
Comment 6 Shinya Kawanaka 2012-10-22 03:30:34 PDT
Created attachment 169869 [details]
Patch
Comment 7 Dimitri Glazkov (Google) 2012-10-22 15:16:46 PDT
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.
Comment 8 Shinya Kawanaka 2012-10-22 19:05:36 PDT
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.
Comment 9 Shinya Kawanaka 2012-10-22 19:21:15 PDT
Created attachment 170043 [details]
Patch
Comment 10 Dimitri Glazkov (Google) 2012-10-22 19:36:12 PDT
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.
Comment 11 Shinya Kawanaka 2012-10-22 19:43:45 PDT
Created attachment 170049 [details]
Patch for landing
Comment 12 Shinya Kawanaka 2012-10-22 19:44:22 PDT
(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.
Comment 13 WebKit Review Bot 2012-10-22 20:08:56 PDT
Comment on attachment 170049 [details]
Patch for landing

Clearing flags on attachment: 170049

Committed r132174: <http://trac.webkit.org/changeset/132174>
Comment 14 WebKit Review Bot 2012-10-22 20:09:00 PDT
All reviewed patches have been landed.  Closing bug.