Title: [211201] trunk
- Revision
- 211201
- Author
- [email protected]
- Date
- 2017-01-25 20:50:01 -0800 (Wed, 25 Jan 2017)
Log Message
Crash under DOMSelection::deleteFromDocument()
https://bugs.webkit.org/show_bug.cgi?id=167232
Reviewed by Chris Dumez.
Source/WebCore:
The crash was caused by DOMSelection's deleteFromDocument() mutating contents inside the user-agent
shadow tree of an input element when the text field is readonly. Fixed the bug by exiting early
whenever the selection is inside a shadow tree since getSelection().getRangeAt(0) always returns
a range outside the input element or any shadow tree for that matter.
New behavior matches that of Gecko. The working draft spec of which I'm the editor states that
deleteFromDocument() must invoke Range's deleteContents() on the associated range, which is
the collapsed range returned by getSelection().getRangeAt(0) in the spec:
https://www.w3.org/TR/2016/WD-selection-api-20160921/#widl-Selection-deleteFromDocument-void
And Range's deleteContents() immediately terminates in step 1 when start and end are identical:
https://dom.spec.whatwg.org/commit-snapshots/6b7621282c2e3b222ac585650e484abf4c0a416b/
Note that Range's DOM mutating methods are not available inside an user-agent shadow tree because
WebKit never returns a Range whose end boundary points are inside the tree to author scripts.
Editing commands (ones executable from document.execCommand) that mutate DOM like this check whether
the content is editable or not. Since VisibleSelection's validate() function makes sure the selection
is either entirely within or outside of an root editable element (editing host in the W3C spec lingo),
editing commands should never mutate a random node inside an user-agent shadow tree.
Test: editing/selection/deleteFromDocument-shadow-tree-crash.html
* page/DOMSelection.cpp:
(WebCore::DOMSelection::deleteFromDocument):
LayoutTests:
Based on a patch by Chris Dumez. Add a regression test and rebaseline a Blink test as WebKit's
new behavior matches that of Gecko instead of Blink.
* editing/selection/deleteFromDocument-shadow-tree-crash-expected.txt: Added.
* editing/selection/deleteFromDocument-shadow-tree-crash.html: Added.
* imported/blink/editing/selection/deleteFromDocument-crash-expected.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (211200 => 211201)
--- trunk/LayoutTests/ChangeLog 2017-01-26 04:24:10 UTC (rev 211200)
+++ trunk/LayoutTests/ChangeLog 2017-01-26 04:50:01 UTC (rev 211201)
@@ -1,3 +1,17 @@
+2017-01-25 Ryosuke Niwa <[email protected]>
+
+ Crash under DOMSelection::deleteFromDocument()
+ https://bugs.webkit.org/show_bug.cgi?id=167232
+
+ Reviewed by Chris Dumez.
+
+ Based on a patch by Chris Dumez. Add a regression test and rebaseline a Blink test as WebKit's
+ new behavior matches that of Gecko instead of Blink.
+
+ * editing/selection/deleteFromDocument-shadow-tree-crash-expected.txt: Added.
+ * editing/selection/deleteFromDocument-shadow-tree-crash.html: Added.
+ * imported/blink/editing/selection/deleteFromDocument-crash-expected.html:
+
2017-01-25 Ryan Haddad <[email protected]>
Marking media/modern-media-controls/airplay-support/airplay-support.html as flaky.
Added: trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash-expected.txt (0 => 211201)
--- trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash-expected.txt 2017-01-26 04:50:01 UTC (rev 211201)
@@ -0,0 +1,3 @@
+This test passes if it does not crash.
+
+
Added: trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash.html (0 => 211201)
--- trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash.html (rev 0)
+++ trunk/LayoutTests/editing/selection/deleteFromDocument-shadow-tree-crash.html 2017-01-26 04:50:01 UTC (rev 211201)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+ function runTest() {
+ document.getElementById('input_0').disabled = true;
+ document.getElementById('input_0').setRangeText("abc");
+ window.getSelection().extend(document.getElementById('input_0'), 0);
+ window.getSelection().deleteFromDocument();
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>This test passes if it does not crash.</p>
+<input id=input_0 type="search">
+</body>
+</html>
Modified: trunk/LayoutTests/imported/blink/editing/selection/deleteFromDocument-crash-expected.html (211200 => 211201)
--- trunk/LayoutTests/imported/blink/editing/selection/deleteFromDocument-crash-expected.html 2017-01-26 04:24:10 UTC (rev 211200)
+++ trunk/LayoutTests/imported/blink/editing/selection/deleteFromDocument-crash-expected.html 2017-01-26 04:50:01 UTC (rev 211201)
@@ -1,2 +1,10 @@
-<textarea autofocus ></textarea>
+<script>
+_onload_ = function() {
+ document.execCommand('selectAll');
+}
+</script>
+<textarea autofocus >
+text
+text2
+text3</textarea>
<iframe srcdoc="foo"></iframe>
Modified: trunk/Source/WebCore/ChangeLog (211200 => 211201)
--- trunk/Source/WebCore/ChangeLog 2017-01-26 04:24:10 UTC (rev 211200)
+++ trunk/Source/WebCore/ChangeLog 2017-01-26 04:50:01 UTC (rev 211201)
@@ -1,3 +1,34 @@
+2017-01-25 Ryosuke Niwa <[email protected]>
+
+ Crash under DOMSelection::deleteFromDocument()
+ https://bugs.webkit.org/show_bug.cgi?id=167232
+
+ Reviewed by Chris Dumez.
+
+ The crash was caused by DOMSelection's deleteFromDocument() mutating contents inside the user-agent
+ shadow tree of an input element when the text field is readonly. Fixed the bug by exiting early
+ whenever the selection is inside a shadow tree since getSelection().getRangeAt(0) always returns
+ a range outside the input element or any shadow tree for that matter.
+
+ New behavior matches that of Gecko. The working draft spec of which I'm the editor states that
+ deleteFromDocument() must invoke Range's deleteContents() on the associated range, which is
+ the collapsed range returned by getSelection().getRangeAt(0) in the spec:
+ https://www.w3.org/TR/2016/WD-selection-api-20160921/#widl-Selection-deleteFromDocument-void
+ And Range's deleteContents() immediately terminates in step 1 when start and end are identical:
+ https://dom.spec.whatwg.org/commit-snapshots/6b7621282c2e3b222ac585650e484abf4c0a416b/
+
+ Note that Range's DOM mutating methods are not available inside an user-agent shadow tree because
+ WebKit never returns a Range whose end boundary points are inside the tree to author scripts.
+ Editing commands (ones executable from document.execCommand) that mutate DOM like this check whether
+ the content is editable or not. Since VisibleSelection's validate() function makes sure the selection
+ is either entirely within or outside of an root editable element (editing host in the W3C spec lingo),
+ editing commands should never mutate a random node inside an user-agent shadow tree.
+
+ Test: editing/selection/deleteFromDocument-shadow-tree-crash.html
+
+ * page/DOMSelection.cpp:
+ (WebCore::DOMSelection::deleteFromDocument):
+
2017-01-25 Ryan Haddad <[email protected]>
Unreviewed, rolling out r211193.
Modified: trunk/Source/WebCore/page/DOMSelection.cpp (211200 => 211201)
--- trunk/Source/WebCore/page/DOMSelection.cpp 2017-01-26 04:24:10 UTC (rev 211200)
+++ trunk/Source/WebCore/page/DOMSelection.cpp 2017-01-26 04:50:01 UTC (rev 211201)
@@ -371,7 +371,7 @@
return;
auto selectedRange = selection.selection().toNormalizedRange();
- if (!selectedRange)
+ if (!selectedRange || selectedRange->shadowRoot())
return;
Ref<Frame> protector(*m_frame);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
