Bug 82010 – [Shadow DOM] InsertionPoint should have isActive() member function.

• Component: DOM
• Status: RESOLVED FIXED

[Hayato Ito]

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.

[Takashi Sakamoto]

Created attachment 135751 [details]
Patch

[Hajime Morrita]

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.

[Takashi Sakamoto]

Created attachment 136174 [details]
Patch

[Takashi Sakamoto]

(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

[WebKit Review Bot]

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

[Takashi Sakamoto]

Created attachment 136619 [details]
Patch

[Takashi Sakamoto]

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

[Hajime Morrita]

Comment on attachment 136619 [details]
Patch

Oops. Sorry for missing this.

[WebKit Review Bot]

Comment on attachment 136619 [details]
Patch

Clearing flags on attachment: 136619

Committed r114334: <http://trac.webkit.org/changeset/114334>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.