Bug 104346

Summary: [Shadow DOM] registering InsertionPoints to ShadowRoot should work out of document tree.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: ojan.autocc, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 104218, 104345    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Shinya Kawanaka
Reported 2012-12-06 23:27:45 PST
Since they are not working out of document tree, ensureDistribution() out of document tree also does not work.
Attachments
Patch (18.36 KB, patch)
2012-12-07 00:12 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (18.45 KB, patch)
2012-12-07 00:17 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (21.67 KB, patch)
2012-12-11 20:37 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch for landing (21.69 KB, patch)
2012-12-11 22:49 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch for landing (21.76 KB, patch)
2012-12-11 23:04 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-12-07 00:12:01 PST
Created attachment 178169 [details] Patch
Shinya Kawanaka
Comment 2 2012-12-07 00:17:27 PST
Created attachment 178170 [details] Patch
Shinya Kawanaka
Comment 3 2012-12-07 01:07:48 PST
Comment on attachment 178170 [details] Patch I found this is wrong. removedFrom() can be called many times (from different TreeScope). We have to check treeScope() when unregistering InsertionPoints.
Shinya Kawanaka
Comment 4 2012-12-07 01:18:39 PST
We also have to add some ASSERT that the same InsertionPoint is not registered many times. Maybe checking treeScope() only is not enough. This is a bit complicated.
Shinya Kawanaka
Comment 5 2012-12-07 01:20:10 PST
We also have to add insertedInto/removedFrom are called many times. It might be worth considering we introduce isInShadowTree flag in InsertionPoint...
Shinya Kawanaka
Comment 6 2012-12-11 20:37:39 PST
Created attachment 178954 [details] Patch
Hajime Morrita
Comment 7 2012-12-11 22:42:17 PST
Comment on attachment 178954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178954&action=review > Source/WebCore/ChangeLog:9 > + out of Document, too. Instead of checking isInDocument(), we have to check InsertionPoint and nit: "check if" or "make sure" or something? For this explanation, you can center when the registration happens. > Source/WebCore/html/shadow/InsertionPoint.cpp:130 > + if (isActive() && !m_registeredWithShadowRoot && insertionPoint->treeScope() == treeScope()) { Nit: root == treeScope()? In that case, It'd better use root instead of treeScope() for comparison here.
Shinya Kawanaka
Comment 8 2012-12-11 22:49:23 PST
Created attachment 178972 [details] Patch for landing
WebKit Review Bot
Comment 9 2012-12-11 22:52:13 PST
Comment on attachment 178972 [details] Patch for landing Rejecting attachment 178972 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: rejects to file LayoutTests/fast/dom/shadow/has-content-elements.html.rej patching file LayoutTests/fast/dom/shadow/has-shadow-insertion-point-expected.txt patching file LayoutTests/fast/dom/shadow/has-shadow-insertion-point.html Hunk #1 FAILED at 11. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/fast/dom/shadow/has-shadow-insertion-point.html.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/15285169
Shinya Kawanaka
Comment 10 2012-12-11 23:04:24 PST
Created attachment 178974 [details] Patch for landing
WebKit Review Bot
Comment 11 2012-12-11 23:23:35 PST
Comment on attachment 178974 [details] Patch for landing Clearing flags on attachment: 178974 Committed r137421: <http://trac.webkit.org/changeset/137421>
WebKit Review Bot
Comment 12 2012-12-11 23:23:39 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.