Bug 80373

Summary: InsertionPoint::attach should be consistent with Element.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 78596    
Attachments:
Description Flags
Patch
none
Patch morrita: review+

Description Shinya Kawanaka 2012-03-05 20:37:32 PST
This bug is preparation for Bug 78596.

InsertionPoint::attach will have code for <shadow> element later, but it is a bit messy now.
So let's clean up the code.

(1) Currently InsertionPoint attaches fallback elements before attaching distributed elements.
To be consistent, attaching distributed elements first is better, since Element attaches shadow tree first.

(2) Let's extract functions from InsertionPoint::attach() also.
Comment 1 Shinya Kawanaka 2012-03-05 20:45:35 PST
Created attachment 130278 [details]
Patch
Comment 2 Hajime Morrita 2012-03-05 21:14:14 PST
Comment on attachment 130278 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Refactoring: Make InsertionPoint code clean.

Could you explain the change more concretely?
What the following explanation says isn't cleanup apparently.

> Source/WebCore/ChangeLog:10
> +        InsertionPoint used to attach fallback element before attaching distributed elements.

fallback elements?

> Source/WebCore/ChangeLog:11
> +        To be consistent with Element::attach behavior, attachign distributed elements first is

s/attachign/attaching/

> Source/WebCore/html/shadow/InsertionPoint.cpp:52
> +        distributeLightChildren(root->tree());

Could you reconsider the name? There is no term "light children" in the standard.
Comment 3 Shinya Kawanaka 2012-03-05 21:34:01 PST
(In reply to comment #2)
> (From update of attachment 130278 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130278&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Refactoring: Make InsertionPoint code clean.
> 
> Could you explain the change more concretely?
> What the following explanation says isn't cleanup apparently.

Done.

> 
> > Source/WebCore/ChangeLog:10
> > +        InsertionPoint used to attach fallback element before attaching distributed elements.
> 
> fallback elements?

Done.

> 
> > Source/WebCore/ChangeLog:11
> > +        To be consistent with Element::attach behavior, attachign distributed elements first is
> 
> s/attachign/attaching/

Done.

> 
> > Source/WebCore/html/shadow/InsertionPoint.cpp:52
> > +        distributeLightChildren(root->tree());
> 
> Could you reconsider the name? There is no term "light children" in the standard.

Renamed to distributeHostChildren.
Comment 4 Shinya Kawanaka 2012-03-05 21:35:00 PST
Created attachment 130280 [details]
Patch
Comment 5 Hajime Morrita 2012-03-05 22:21:13 PST
Comment on attachment 130280 [details]
Patch

looks good.
Comment 6 Shinya Kawanaka 2012-03-05 22:59:29 PST
Committed r109864: <http://trac.webkit.org/changeset/109864>