Bug 149109

Summary: Improve Node pre-insertion validation when the parent is a Document
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, esprehn+autocc, kangil.han, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews104 for mac-mavericks-wk2
none
Patch none

Description Chris Dumez 2015-09-13 20:09:43 PDT
Improve Node pre-insertion validation when the parent is a Document to match the specification:
https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
https://dom.spec.whatwg.org/#concept-node-replace

This affects the following API: Node.insertBefore(), Node.appendChild(), Node.replaceChild().

WebKit current fails to do the following checks whenever the parent is a Document from pre-insertion validation:
1. If the inserted Node is a DocumentFragment, we should make sure it contains only one Element.
-> This is because a Document can have only one child that is an Element.
2.a. If an Element is inserted, we should make sure it is not inserted before a DocumentType.
2.b. If a DocumentType is inserted, we should make sure it is not inserted after an Element.
-> This is because the DocType must come before the optional Element child as per https://dom.spec.whatwg.org/#node-trees

Firefox and Chrome already match the specification here.
Comment 1 Chris Dumez 2015-09-13 20:10:18 PDT
rdar://problem/22560436
Comment 2 Chris Dumez 2015-09-13 20:17:51 PDT
Created attachment 261093 [details]
Patch
Comment 3 Build Bot 2015-09-13 21:05:28 PDT
Comment on attachment 261093 [details]
Patch

Attachment 261093 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/169071

New failing tests:
userscripts/mixed-case-stylesheet.html
Comment 4 Build Bot 2015-09-13 21:05:33 PDT
Created attachment 261096 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 5 Chris Dumez 2015-09-13 21:15:18 PDT
Created attachment 261097 [details]
Patch
Comment 6 Ryosuke Niwa 2015-09-13 21:17:44 PDT
Comment on attachment 261097 [details]
Patch

r=me assuming all tests pass and there is no performance regression from this change
(check Dromaeo & Speedometer).
Comment 7 Chris Dumez 2015-09-13 21:22:58 PDT
(In reply to comment #6)
> Comment on attachment 261097 [details]
> Patch
> 
> r=me assuming all tests pass and there is no performance regression from
> this change
> (check Dromaeo & Speedometer).

Ok, I'll check the benchmarks before landing. I wasn't too worried because this change only affects inserting a direct child to the Document. You cannot insert a lot of those, especially Doctype / Element (just one of each).
Comment 8 Chris Dumez 2015-09-13 22:08:51 PDT
Comment on attachment 261097 [details]
Patch

Clearing flags on attachment: 261097

Committed r189682: <http://trac.webkit.org/changeset/189682>
Comment 9 Chris Dumez 2015-09-13 22:08:58 PDT
All reviewed patches have been landed.  Closing bug.