| Summary: | Appending ShadowRoot into an element should not cause crash. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Shinya Kawanaka <shinyak> | ||||||||
| Component: | DOM | Assignee: | Shinya Kawanaka <shinyak> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | dglazkov, dominicc, hayato, jamesr, morrita, rolandsteiner, shinyak, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
I think we have two options in this case: A). Throws DOM Exception B). ShadowRoot acts like as DOCUMENT_FRAGMENT so every children of shadowRoot are appended. It seems the spec says B implicitly because ShadowRoot's nodeType is DOCUMENT_FRAGMENT_NODE. I thinks A is safer option, but we should file a bug for the spec if we choose A. I've filed a bug here. https://www.w3.org/Bugs/Public/show_bug.cgi?id=16130 Created attachment 128953 [details]
Patch
(In reply to comment #1) > I think we have two options in this case: > > A). Throws DOM Exception > B). ShadowRoot acts like as DOCUMENT_FRAGMENT so every children of shadowRoot are appended. > > It seems the spec says B implicitly because ShadowRoot's nodeType is DOCUMENT_FRAGMENT_NODE. > I thinks A is safer option, but we should file a bug for the spec if we choose A. I wrote a patch to reject appending shadow root. If we should choose (B) option, I'll change the patch. Comment on attachment 128953 [details]
Patch
It should act as a document fragment.
(In reply to comment #5) > (From update of attachment 128953 [details]) > It should act as a document fragment. OK. Thnaks, Dimitri. I'll update the patch. Created attachment 129624 [details]
Patch
Comment on attachment 129624 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129624&action=review > LayoutTests/fast/dom/shadow/shadow-root-append.html:25 > + shouldBeNull('root.firstChild'); Should you be checking to see if the container has chlidren now? Woah. the watchlist thought this patch touched Source/WebCore/platform/graphics/chromium/*. very weird! (In reply to comment #8) > (From update of attachment 129624 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=129624&action=review > > > LayoutTests/fast/dom/shadow/shadow-root-append.html:25 > > + shouldBeNull('root.firstChild'); > > Should you be checking to see if the container has chlidren now? Sure. Ah, I found that I missed adding Skipped list. I'll land this patch with Skipped list. Created attachment 129625 [details]
Patch for landing
Comment on attachment 129625 [details] Patch for landing Rejecting attachment 129625 [details] from commit-queue. shinyak@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Comment on attachment 129625 [details] Patch for landing Clearing flags on attachment: 129625 Committed r109359: <http://trac.webkit.org/changeset/109359> All reviewed patches have been landed. Closing bug. |
This will cause an assertion failure. var div1 = document.createElement('div'); var root = new WebKithShadowRoot(div1); var div2 = document.createElement('div'); div2.appendChild(root);