Selection.deleteFromDocument should not leave a selection character
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Apr 2016 08:53:38 +0000 (08:53 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Apr 2016 08:53:38 +0000 (08:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151442

Reviewed by Michael Catanzaro.

Source/WebCore:

This is a merge of Blink r172511:
https://codereview.chromium.org/255453003

Let Selection.deleteFromDocument not delete a character when the
selection is a caret.

Selection.deleteFromDocument delete a character when the selection
is a caret.
However, current standard says that Selection.deleteFromDocument
does nothing when the selection is a caret:
https://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#dom-selection-deletefromdocument
Both IE10 and FireFox seem following the spec.

Test: imported/blink/editing/selection/deleteFromDocument-undo-crash.html

* page/DOMSelection.cpp:
(WebCore::DOMSelection::deleteFromDocument): Deleted.

LayoutTests:

* editing/selection/deleteFromDocument-expected.txt:
* editing/selection/deleteFromDocument.html:
* imported/blink/editing/selection/deleteFromDocument-undo-crash-expected.txt: Added.
* imported/blink/editing/selection/deleteFromDocument-undo-crash.html: Added.
* TestExpectations: Skip imported/blink/editing/selection/deleteFromDocument-undo-crash.html in Debug.
* platform/gtk/TestExpectations: Unskip imported/blink/editing/selection/deleteFromDocument-crash.html.
* platform/win/TestExpectations: Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199585 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/editing/selection/deleteFromDocument-expected.txt
LayoutTests/editing/selection/deleteFromDocument.html
LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash-expected.txt [new file with mode: 0644]
LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash.html [new file with mode: 0644]
LayoutTests/platform/gtk/TestExpectations
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/DOMSelection.cpp

index d820359..cdd5c7b 100644 (file)
@@ -1,3 +1,18 @@
+2016-04-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Selection.deleteFromDocument should not leave a selection character
+        https://bugs.webkit.org/show_bug.cgi?id=151442
+
+        Reviewed by Michael Catanzaro.
+
+        * editing/selection/deleteFromDocument-expected.txt:
+        * editing/selection/deleteFromDocument.html:
+        * imported/blink/editing/selection/deleteFromDocument-undo-crash-expected.txt: Added.
+        * imported/blink/editing/selection/deleteFromDocument-undo-crash.html: Added.
+        * TestExpectations: Skip imported/blink/editing/selection/deleteFromDocument-undo-crash.html in Debug.
+        * platform/gtk/TestExpectations: Unskip imported/blink/editing/selection/deleteFromDocument-crash.html.
+        * platform/win/TestExpectations: Ditto.
+
 2016-04-15  Brady Eidson  <beidson@apple.com>
 
         Add the message property to DOMError.
index 18f9545..7ba2efa 100644 (file)
@@ -471,6 +471,7 @@ webkit.org/b/139595 http/tests/xmlhttprequest/workers/abort-exception-assert.htm
 [ Debug ] fast/history/history-back-while-pdf-in-pagecache.html [ Skip ]
 webkit.org/b/121628 [ Release ] fast/history/history-back-while-pdf-in-pagecache.html [ Pass ImageOnlyFailure ]
 [ Debug ] fullscreen/full-screen-iframe-legacy.html [ Pass Crash ]
+webkit.org/b/151488 [ Debug ] imported/blink/editing/selection/deleteFromDocument-undo-crash.html [ Skip ]
 
 webkit.org/b/139634 [ Debug ] fast/selectors/matches-backtracking.html [ Slow ]
 webkit.org/b/139634 [ Debug ] fast/selectors/nth-child-of-register-requirement.html [ Slow ]
index f68ac2b..151cbeb 100644 (file)
@@ -1,7 +1,9 @@
+PASS span1.textContent is "foobar"
+PASS span1.textContent is "foobar"
+PASS span1.firstChild.nextSibling.id is "span2"
+PASS span1.firstChild.nextSibling.firstChild.nodeValue is ""
+PASS test.textContent is "fz"
+PASS successfullyParsed is true
+
+TEST COMPLETE
 fz
-Success: span1.textContent is fooar.
-Success: span1.textContent is foar.
-Success: span1.textContent is foar.
-Success: span1.firstChild.nextSibling.id is span2.
-Success: span1.firstChild.nextSibling.firstChild.nodeValue is .
-Success: test.textContent is fz.
index 1a94a20..851dc8b 100644 (file)
@@ -1,7 +1,9 @@
 <html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
 <body>
 <div id="test"><span id="span1">foo<span id="span2">bar</span></span><span id="span3">baz</span></div>
-<div id="console"></div>
 <script>
 var s = window.getSelection();
 var testDiv = document.getElementById("test");
@@ -9,23 +11,6 @@ var span1 = document.getElementById("span1");
 var span2 = document.getElementById("span2");
 var span3 = document.getElementById("span3");
 
-function log(str) {
-    var li = document.createElement("li");
-    li.appendChild(document.createTextNode(str));
-    document.getElementById("console").appendChild(li);
-}
-
-function shouldBe(expr, expected) {
-    var actual = eval(expr);
-    if (actual != expected)
-        log("Failure: " + expr + " should be " + expected + ", was " + actual + ".");
-    else
-        log("Success: " + expr + " is " + expected + ".");
-}
-
-if (window.testRunner)
-    testRunner.dumpAsText();
-
 var r = document.createRange();
 
 
@@ -35,14 +20,7 @@ r.setEnd(span2.firstChild, 1);
 s.addRange(r);
 
 s.deleteFromDocument()
-shouldBe('span1.textContent', 'fooar');
-try {
-  s.deleteFromDocument()
-  shouldBe('span1.textContent', 'foar');
-} catch (ex) {
-  log(ex);
-}
-
+shouldBeEqualToString("span1.textContent", "foobar");
 
 // try to delete a collapsed selection at the start
 r.setStart(span1.firstChild, 0);
@@ -50,24 +28,18 @@ r.setEnd(span1.firstChild, 0);
 s.removeAllRanges();
 s.addRange(r);
 
-try {
-  s.deleteFromDocument()
-  shouldBe('span1.textContent', 'foar');
-} catch (ex) {
-  log(ex);
-}
-
+s.deleteFromDocument()
+shouldBeEqualToString("span1.textContent", "foobar");
 
 // delete whole contents
 r.setStart(span2.firstChild, 0);
-r.setEnd(span2.firstChild, 2);
+r.setEnd(span2.firstChild, 3);
 s.removeAllRanges();
 s.addRange(r);
 
 s.deleteFromDocument()
-shouldBe('span1.firstChild.nextSibling.id', 'span2');
-shouldBe('span1.firstChild.nextSibling.firstChild.nodeValue', '');
-
+shouldBeEqualToString("span1.firstChild.nextSibling.id", "span2");
+shouldBeEqualToString("span1.firstChild.nextSibling.firstChild.nodeValue", "");
 
 // partially delete two nodes
 r.setStart(span1.firstChild, 1);
@@ -76,7 +48,7 @@ s.removeAllRanges();
 s.addRange(r);
 
 s.deleteFromDocument()
-shouldBe('test.textContent', 'fz');
+shouldBeEqualToString("test.textContent", "fz");
 
 </script>
 </body>
diff --git a/LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash-expected.txt b/LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash-expected.txt
new file mode 100644 (file)
index 0000000..2afa0bf
--- /dev/null
@@ -0,0 +1 @@
+PASS. WebKit didn't crash.
diff --git a/LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash.html b/LayoutTests/imported/blink/editing/selection/deleteFromDocument-undo-crash.html
new file mode 100644 (file)
index 0000000..4b388ae
--- /dev/null
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script id="script1"></script>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    document.designMode = "on";
+    var selection = window.getSelection();
+    document.execCommand("SelectAll", false)
+    var range = selection.getRangeAt(0);
+
+    range.insertNode(document.getElementById("script1"));
+    document.execCommand('InsertText', false, 'efgh');
+    document.execCommand('JustifyFull', false, false);
+
+    selection.deleteFromDocument();
+    document.execCommand("Undo", false, false);
+
+    document.execCommand("Delete");
+
+    document.documentElement.innerHTML = "PASS. WebKit didn't crash.";
+};
+</script>
+</head>
+<body>
+    <h1>abcd</h1>
+</body>
+</html>
index 3c1fc87..69baee5 100644 (file)
@@ -2467,7 +2467,6 @@ webkit.org/b/151267 imported/blink/css3/blending/mix-blend-mode-with-squashing-l
 webkit.org/b/151267 imported/blink/css3/blending/svg-blend-overlapping-elements.html [ ImageOnlyFailure ]
 webkit.org/b/151267 imported/blink/css3/blending/svg-isolation-foreign-no-isolation.html [ ImageOnlyFailure ]
 webkit.org/b/151267 imported/blink/css3/blending/svg-isolation-nested-svg-no-isolation.html [ ImageOnlyFailure ]
-webkit.org/b/151267 imported/blink/editing/selection/deleteFromDocument-crash.html [ ImageOnlyFailure ]
 webkit.org/b/151267 imported/blink/fast/canvas/canvas-clip-stack-persistence.html [ ImageOnlyFailure ]
 webkit.org/b/151267 imported/blink/fast/css/fixed-overlaps-absolute-in-clip.html [ ImageOnlyFailure Pass ]
 webkit.org/b/151267 imported/blink/fast/css/tab-size-complex-path.html [ ImageOnlyFailure ]
index 2d46b58..c7f3271 100644 (file)
@@ -3281,7 +3281,6 @@ imported/blink/css3/blending/svg-isolation-foreign-no-isolation.html [ ImageOnly
 imported/blink/css3/blending/svg-isolation-nested-svg-no-isolation.html [ ImageOnlyFailure ]
 imported/blink/css3/filters/effect-drop-shadow-clip-abspos.html [ ImageOnlyFailure ]
 imported/blink/css3/filters/effect-reference-on-transparent-element.html [ ImageOnlyFailure ]
-imported/blink/editing/selection/deleteFromDocument-crash.html [ ImageOnlyFailure ]
 imported/blink/fast/css/tab-size-complex-path.html [ ImageOnlyFailure ]
 imported/blink/fast/css/text-decoration-propagate.html [ ImageOnlyFailure ]
 imported/blink/fast/text/emoji-font-fallback-win.html [ ImageOnlyFailure ]
index 3c68ea0..0cacb55 100644 (file)
@@ -1,3 +1,28 @@
+2016-04-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Selection.deleteFromDocument should not leave a selection character
+        https://bugs.webkit.org/show_bug.cgi?id=151442
+
+        Reviewed by Michael Catanzaro.
+
+        This is a merge of Blink r172511:
+        https://codereview.chromium.org/255453003
+
+        Let Selection.deleteFromDocument not delete a character when the
+        selection is a caret.
+
+        Selection.deleteFromDocument delete a character when the selection
+        is a caret.
+        However, current standard says that Selection.deleteFromDocument
+        does nothing when the selection is a caret:
+        https://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#dom-selection-deletefromdocument
+        Both IE10 and FireFox seem following the spec.
+
+        Test: imported/blink/editing/selection/deleteFromDocument-undo-crash.html
+
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::deleteFromDocument): Deleted.
+
 2016-04-15  Antti Koivisto  <antti@apple.com>
 
         Fix return value nullptr -> false.
index a37f99d..e119f64 100644 (file)
@@ -429,9 +429,6 @@ void DOMSelection::deleteFromDocument()
     if (selection.isNone())
         return;
 
-    if (isCollapsed())
-        selection.modify(FrameSelection::AlterationExtend, DirectionBackward, CharacterGranularity);
-
     RefPtr<Range> selectedRange = selection.selection().toNormalizedRange();
     if (!selectedRange)
         return;