Summary: | [Shadow DOM] InsertionPoint should have isActive() member function. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hayato Ito <hayato> | ||||||||
Component: | DOM | Assignee: | 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
Hayato Ito
2012-03-22 21:16:34 PDT
Created attachment 135751 [details]
Patch
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. Created attachment 136174 [details]
Patch
(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 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 Created attachment 136619 [details]
Patch
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 on attachment 136619 [details]
Patch
Oops. Sorry for missing this.
Comment on attachment 136619 [details] Patch Clearing flags on attachment: 136619 Committed r114334: <http://trac.webkit.org/changeset/114334> All reviewed patches have been landed. Closing bug. |