Bug 149109 - Improve Node pre-insertion validation when the parent is a Document
Summary: Improve Node pre-insertion validation when the parent is a Document
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://dom.spec.whatwg.org/#concept-...
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2015-09-13 20:09 PDT by Chris Dumez
Modified: 2015-09-13 22:08 PDT (History)
7 users (show)

See Also:


Attachments
Patch (21.49 KB, patch)
2015-09-13 20:17 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (658.96 KB, application/zip)
2015-09-13 21:05 PDT, Build Bot
no flags Details
Patch (22.13 KB, patch)
2015-09-13 21:15 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.