New issue
Advanced search Search tips

Issue 699861 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Range.surroundContents() should not reject an ancestor of the Range as newParent

Project Member Reported by tkent@chromium.org, Mar 9 2017

Issue description

Chrome Version: 59 canary
OS: All but iOS

What steps will reproduce the problem?
(1) Open the following HTML.

<!DOCTYPE html>
<div>bar</div>
<script>
var r = document.createRange();
var d = document.querySelector('div');
var t = d.firstChild;
r.setStart(t, 1);
r.setEnd(t, 2);
r.surroundContents(d);
</script>

What is the expected result?
HierarchyRequestError is thrown at the step 5 of https://dom.spec.whatwg.org/#dom-range-surroundcontents . So, the content of <div> is empty, 

What happens instead?
HierarchyRequestError is thrown before the step 3. So the document is unchanged.

Please use labels and text to provide additional information.
Firefox: OK
Safari: OK
Edge: NG, same as Chrome.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

 
Summary: Range.surroundContents() should not reject an ancestor of the Range as newParent (was: Range.sorroundContents() should not reject an ancestor of the Range as newParent)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 10 2017

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

commit a3c94e4cc81caa2693ee56579e74c71e2fcadb0f
Author: tkent <tkent@chromium.org>
Date: Fri Mar 10 01:14:54 2017

DOM Range: surroundContents() should not check if newParent is in ancestors beforehand.

https://dom.spec.whatwg.org/#dom-range-surroundcontents
According to the DOM standard, we should not check it before starting DOM
mutation. Step 5 throws HierarchyRequestError instead.

This CL fixes 44 tests in wpt/dom/ranges/Range-surroundContents.html.

* images/element-gcd-while-generating-alt-content.html: Removed.
  This CL changes behavior of the test. The test was for a crash bug in
  pre-Olilpan era, and we don't have code for the crash fix any longer.

* fast/dom/Range/surroundContents-1.html: Removed.
  This is a pixel test to verify HierarchyRequestError which this CL removes.
  This behavior is tested well in WPT.

* shadow-dom/range-surround-contents.html: Updated
  DOM trees are modified because this CL removes a precondition check.

BUG= 699861 

Review-Url: https://codereview.chromium.org/2745443003
Cr-Commit-Position: refs/heads/master@{#455941}

[modify] https://crrev.com/a3c94e4cc81caa2693ee56579e74c71e2fcadb0f/third_party/WebKit/LayoutTests/external/wpt/dom/ranges/Range-surroundContents-expected.txt
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/fast/dom/Range/surroundContents-1.html
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content.html
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/linux/fast/dom/Range/surroundContents-1-expected.png
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/linux/fast/dom/Range/surroundContents-1-expected.txt
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/mac/fast/dom/Range/surroundContents-1-expected.png
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/mac/fast/dom/Range/surroundContents-1-expected.txt
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/win/fast/dom/Range/surroundContents-1-expected.png
[delete] https://crrev.com/337642ea1516b47bd972b0f980a0a71b19931dd2/third_party/WebKit/LayoutTests/platform/win/fast/dom/Range/surroundContents-1-expected.txt
[modify] https://crrev.com/a3c94e4cc81caa2693ee56579e74c71e2fcadb0f/third_party/WebKit/LayoutTests/shadow-dom/range-surround-contents.html
[modify] https://crrev.com/a3c94e4cc81caa2693ee56579e74c71e2fcadb0f/third_party/WebKit/Source/core/dom/Range.cpp

Comment 3 by tkent@chromium.org, Mar 10 2017

Labels: M-59
Status: Fixed (was: Started)

Sign in to add a comment