Bug 82010

Summary: [Shadow DOM] InsertionPoint should have isActive() member function.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, morrita, shinyak, tasak, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77499    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Hayato Ito 2012-03-22 21:16:34 PDT
According to the Shadow DOM spec, each InsertionPoint is either 'active' or 'inactive'.
it will be useful if InsertionPoint has isActive() member function so that we can know it easily.

One use case is ReifiedTreeTraversal. See the comment on https://bugs.webkit.org/show_bug.cgi?id=79197#c34.
Comment 1 Takashi Sakamoto 2012-04-04 20:54:06 PDT
Created attachment 135751 [details]
Patch
Comment 2 Hajime Morrita 2012-04-08 17:40:02 PDT
Comment on attachment 135751 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Shadow DOM] InsertionPoint should have isActive() member function.

This change doesn't look just adding a member function. It also affects the behavior.
Could you reflect that fact in this summary?

Also for the record, could you have a link to the spec in the changelog?

> Source/WebCore/html/shadow/InsertionPoint.cpp:51
> +    if (isShadowBoundary() && isActive()) {

My feeling is that isShadowBoundary() should contains isActive() since inactive node cannot be a boundary.
Comment 3 Takashi Sakamoto 2012-04-08 20:41:36 PDT
Created attachment 136174 [details]
Patch
Comment 4 Takashi Sakamoto 2012-04-08 20:45:13 PDT
(In reply to comment #2)
> (From update of attachment 135751 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135751&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        [Shadow DOM] InsertionPoint should have isActive() member function.
> 
> This change doesn't look just adding a member function. It also affects the behavior.
> Could you reflect that fact in this summary?

I see. I updated the summary. 

> 
> Also for the record, could you have a link to the spec in the changelog?

I added a link, https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#dfn-active-insertion-point.

> 
> > Source/WebCore/html/shadow/InsertionPoint.cpp:51
> > +    if (isShadowBoundary() && isActive()) {
> 
> My feeling is that isShadowBoundary() should contains isActive() since inactive node cannot be a boundary.

I see. I added isActive check to isShadowBoundary and removed from attach().

Best regards,
Takashi Sakamoto
Comment 5 WebKit Review Bot 2012-04-10 18:48:52 PDT
Comment on attachment 136174 [details]
Patch

Rejecting attachment 136174 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: ChangeLog entry in Source/WebCore/ChangeLog is not at the top of the file.

Full output: http://queues.webkit.org/results/12383553
Comment 6 Takashi Sakamoto 2012-04-10 21:44:06 PDT
Created attachment 136619 [details]
Patch
Comment 7 Takashi Sakamoto 2012-04-10 21:44:40 PDT
I modified patch for fixing the following issue:
ERROR: ChangeLog entry in Source/WebCore/ChangeLog is not at the top of the file.

Best regards,
Takashi Sakamoto
Comment 8 Hajime Morrita 2012-04-16 19:09:15 PDT
Comment on attachment 136619 [details]
Patch

Oops. Sorry for missing this.
Comment 9 WebKit Review Bot 2012-04-16 19:55:58 PDT
Comment on attachment 136619 [details]
Patch

Clearing flags on attachment: 136619

Committed r114334: <http://trac.webkit.org/changeset/114334>
Comment 10 WebKit Review Bot 2012-04-16 19:56:03 PDT
All reviewed patches have been landed.  Closing bug.