New issue
Advanced search Search tips

Issue 729432 link

Starred by 1 user
Status: Fixed
Owner:
tkent@chromium.org
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Node's insertBefore(), replaceChild(), and appendChild() should check DocumentType position

Project Member Reported by tkent@chromium.org, Jun 5 2017 
Chrome Version: 61 canary
OS: All but iOS

What steps will reproduce the problem?
(1) Open http://w3c-test.org/dom/nodes/Node-insertBefore.html or http://w3c-test.org/dom/nodes/Node-replaceChild.html
(2) Obsere

What is the expected result?
No Fails.

What happens instead?
Some fails.

Please use labels and text to provide additional information.

https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
We don't implement step 6 correctly.

↪ element
  parent has an element child, child is a doctype, or child is not null and a doctype is following child.
↪ doctype
  parent has a doctype child, child is non-null and an element is preceding child, or child is null and parent has an element child.

Safari TP: OK
Firefox nightly: OK
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f5c0dba3c72ae92899db2b355ca36c9da7b49fb

commit 5f5c0dba3c72ae92899db2b355ca36c9da7b49fb
Author: Kent Tamura <tkent@chromium.org>
Date: Tue Jun 06 08:21:50 2017

DOM: skip re-check for invalid node in mutation operations.

In appendChild(), insertBefore(), and replaceChild(), we checked input
node validity twice because internal removeChild() call dispatches
synchronous events and event handlers may alter DOM structure
unexpectedly. This CL make the second validation skippable
by detecting DOM tree modification.

This is a preparation to fix  crbug.com/729432 .

Bug:  729432 
Change-Id: Idccc91f2e18deec488f4820b2448681199e412ba
Reviewed-on: https://chromium-review.googlesource.com/523682
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477232}
[modify] https://crrev.com/5f5c0dba3c72ae92899db2b355ca36c9da7b49fb/third_party/WebKit/Source/core/dom/ContainerNode.cpp
[modify] https://crrev.com/5f5c0dba3c72ae92899db2b355ca36c9da7b49fb/third_party/WebKit/Source/core/dom/ContainerNode.h
[modify] https://crrev.com/5f5c0dba3c72ae92899db2b355ca36c9da7b49fb/third_party/WebKit/Source/core/dom/DocumentTest.cpp
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/121fec7bb384638df1cc3e26fda0bed886970a38

commit 121fec7bb384638df1cc3e26fda0bed886970a38
Author: Kent Tamura <tkent@chromium.org>
Date: Wed Jun 07 06:36:29 2017

Document's insertBefore(), replaceChild(), and appendChild() should check DocumentType position

According to the DOM standard, we should reject to insert an element before a
doctype, and insert a doctype after an element.

The new behavior matches to Firefox and Safari.

Bug:  729432 
Change-Id: I61ef6e7604c829a4248a7befe6e4442667dc2ec4
Reviewed-on: https://chromium-review.googlesource.com/526557
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477563}
[delete] https://crrev.com/613581c6d2fac5deb21685714964ee1f2f71f36f/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Node-insertBefore-expected.txt
[delete] https://crrev.com/613581c6d2fac5deb21685714964ee1f2f71f36f/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Node-replaceChild-expected.txt
[modify] https://crrev.com/121fec7bb384638df1cc3e26fda0bed886970a38/third_party/WebKit/Source/core/dom/ContainerNode.cpp
[modify] https://crrev.com/121fec7bb384638df1cc3e26fda0bed886970a38/third_party/WebKit/Source/core/dom/ContainerNode.h
[modify] https://crrev.com/121fec7bb384638df1cc3e26fda0bed886970a38/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/121fec7bb384638df1cc3e26fda0bed886970a38/third_party/WebKit/Source/core/dom/Document.h

Comment 3 by tkent@chromium.org, Jun 7 2017

Labels: M-61
Status: Fixed (was: Assigned)
►

Sign in to add a comment