Align Selection API with the specification
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Aug 2016 20:30:29 +0000 (20:30 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Aug 2016 20:30:29 +0000 (20:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160663

Reviewed by Ryosuke Niwa.

Source/WebCore:

Align Selection API with the specification:
- https://www.w3.org/TR/selection-api/#idl-def-Selection

In particular, the following changes were made:
- Mark parameters as non-nullable when they should be.
- Mark parameters as mandatory when they should be.
- Use "unsigned long" type for offsets instead of "long".

This aligns our behavior with Firefox and Chrome.

Note that the Node parameters to setBaseAndExtent() operation were kept
nullable, which does not match the specification. This is intentional
as I worry about compatibility risk, especially considering they are
still nullable in Chrome. Only Firefox marks them as non-nullable.

Test: editing/selection/bad-input.html

* dom/Position.h:
(WebCore::Position::LegacyEditingOffset::value):
(WebCore::Position::LegacyEditingOffset::LegacyEditingOffset):
(WebCore::createLegacyEditingPosition):
* page/DOMSelection.cpp:
(WebCore::DOMSelection::anchorOffset):
(WebCore::DOMSelection::focusOffset):
(WebCore::DOMSelection::baseOffset):
(WebCore::DOMSelection::extentOffset):
(WebCore::DOMSelection::rangeCount):
(WebCore::DOMSelection::collapse):
(WebCore::DOMSelection::setBaseAndExtent):
(WebCore::DOMSelection::setPosition):
(WebCore::DOMSelection::extend):
(WebCore::DOMSelection::getRangeAt):
(WebCore::DOMSelection::addRange):
(WebCore::DOMSelection::deleteFromDocument):
(WebCore::DOMSelection::containsNode):
(WebCore::DOMSelection::selectAllChildren):
(WebCore::DOMSelection::shadowAdjustedOffset):
(WebCore::DOMSelection::modify): Deleted.
(WebCore::DOMSelection::shadowAdjustedNode): Deleted.
(WebCore::DOMSelection::isValidForPosition): Deleted.
* page/DOMSelection.h:
* page/DOMSelection.idl:

LayoutTests:

* editing/selection/bad-input-expected.txt: Added.
* editing/selection/bad-input.html: Added.
Add new layout test to cover passing bad input to the Selection API.
This new test is passing completely in Firefox and Chrome.

* editing/execCommand/apply-style-text-decoration-crash.html:
* editing/execCommand/applyblockelement-visiblepositionforindex-crash.html:
* editing/execCommand/ident-crashes-topnode-is-text.html:
* editing/execCommand/indent-pre-expected.txt:
* editing/execCommand/indent-pre.html:
* editing/execCommand/overtype.html:
* editing/selection/containsNode-expected.txt:
* editing/selection/containsNode.html:
* editing/selection/move-by-line-003.html:
* editing/selection/script-tests/DOMSelection-DocumentType.js:
* editing/selection/script-tests/DOMSelection-crossing-document.js:
(clear):
* editing/selection/selection-invalid-offset-expected.txt:
* fast/block/float/float-list-changed-before-layout-crash.html:
* fast/dom/non-numeric-values-numeric-parameters-expected.txt:
* fast/dom/script-tests/non-numeric-values-numeric-parameters.js:
* fast/events/selectstart-by-arrow-keys.html:
* fast/html/nav-element.html:
* fast/html/script-tests/article-element.js:
* fast/html/script-tests/aside-element.js:
* fast/html/script-tests/footer-element.js:
* fast/html/script-tests/header-element.js:
* fast/html/script-tests/hgroup-element.js:
* fast/html/script-tests/main-element.js:
* fast/html/script-tests/section-element.js:
* imported/blink/accessibility/event-on-deleted-iframe-causes-crash.html:
* imported/blink/editing/apply-inline-style-to-element-with-no-renderer-crash.html:
* svg/custom/unicode-in-tspan-multi-svg-crash.html:
Update existing tests to use the Selection API properly.

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

36 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/apply-style-text-decoration-crash.html
LayoutTests/editing/execCommand/applyblockelement-visiblepositionforindex-crash.html
LayoutTests/editing/execCommand/ident-crashes-topnode-is-text.html
LayoutTests/editing/execCommand/indent-pre-expected.txt
LayoutTests/editing/execCommand/indent-pre.html
LayoutTests/editing/execCommand/overtype.html
LayoutTests/editing/pasteboard/resources/select-and-drag.js
LayoutTests/editing/selection/bad-input-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/bad-input.html [new file with mode: 0644]
LayoutTests/editing/selection/containsNode-expected.txt
LayoutTests/editing/selection/containsNode.html
LayoutTests/editing/selection/move-by-line-003.html
LayoutTests/editing/selection/script-tests/DOMSelection-DocumentType.js
LayoutTests/editing/selection/script-tests/DOMSelection-crossing-document.js
LayoutTests/editing/selection/selection-invalid-offset-expected.txt
LayoutTests/fast/block/float/float-list-changed-before-layout-crash.html
LayoutTests/fast/dom/non-numeric-values-numeric-parameters-expected.txt
LayoutTests/fast/dom/script-tests/non-numeric-values-numeric-parameters.js
LayoutTests/fast/events/selectstart-by-arrow-keys.html
LayoutTests/fast/html/nav-element.html
LayoutTests/fast/html/script-tests/article-element.js
LayoutTests/fast/html/script-tests/aside-element.js
LayoutTests/fast/html/script-tests/footer-element.js
LayoutTests/fast/html/script-tests/header-element.js
LayoutTests/fast/html/script-tests/hgroup-element.js
LayoutTests/fast/html/script-tests/main-element.js
LayoutTests/fast/html/script-tests/section-element.js
LayoutTests/imported/blink/accessibility/event-on-deleted-iframe-causes-crash.html
LayoutTests/imported/blink/editing/apply-inline-style-to-element-with-no-renderer-crash.html
LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Position.h
Source/WebCore/page/DOMSelection.cpp
Source/WebCore/page/DOMSelection.h
Source/WebCore/page/DOMSelection.idl

index 559f6a5..27b47e1 100644 (file)
@@ -1,3 +1,45 @@
+2016-08-08  Chris Dumez  <cdumez@apple.com>
+
+        Align Selection API with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=160663
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/selection/bad-input-expected.txt: Added.
+        * editing/selection/bad-input.html: Added.
+        Add new layout test to cover passing bad input to the Selection API.
+        This new test is passing completely in Firefox and Chrome.
+
+        * editing/execCommand/apply-style-text-decoration-crash.html:
+        * editing/execCommand/applyblockelement-visiblepositionforindex-crash.html:
+        * editing/execCommand/ident-crashes-topnode-is-text.html:
+        * editing/execCommand/indent-pre-expected.txt:
+        * editing/execCommand/indent-pre.html:
+        * editing/execCommand/overtype.html:
+        * editing/selection/containsNode-expected.txt:
+        * editing/selection/containsNode.html:
+        * editing/selection/move-by-line-003.html:
+        * editing/selection/script-tests/DOMSelection-DocumentType.js:
+        * editing/selection/script-tests/DOMSelection-crossing-document.js:
+        (clear):
+        * editing/selection/selection-invalid-offset-expected.txt:
+        * fast/block/float/float-list-changed-before-layout-crash.html:
+        * fast/dom/non-numeric-values-numeric-parameters-expected.txt:
+        * fast/dom/script-tests/non-numeric-values-numeric-parameters.js:
+        * fast/events/selectstart-by-arrow-keys.html:
+        * fast/html/nav-element.html:
+        * fast/html/script-tests/article-element.js:
+        * fast/html/script-tests/aside-element.js:
+        * fast/html/script-tests/footer-element.js:
+        * fast/html/script-tests/header-element.js:
+        * fast/html/script-tests/hgroup-element.js:
+        * fast/html/script-tests/main-element.js:
+        * fast/html/script-tests/section-element.js:
+        * imported/blink/accessibility/event-on-deleted-iframe-causes-crash.html:
+        * imported/blink/editing/apply-inline-style-to-element-with-no-renderer-crash.html:
+        * svg/custom/unicode-in-tspan-multi-svg-crash.html:
+        Update existing tests to use the Selection API properly.
+
 2016-08-08  John Wilander  <wilander@apple.com>
 
         Don't set document.domain to an IP address fragment
index fccb478..46bbcb9 100644 (file)
@@ -5,7 +5,7 @@
             if (window.testRunner)\r
                 testRunner.dumpAsText();\r
 \r
-            window.getSelection().setBaseAndExtent(start, 0, null, 0);\r
+            window.getSelection().setBaseAndExtent(start, 0, start, 0);\r
             document.execCommand("Indent");\r
             document.getElementById("result").innerHTML = "PASS";\r
         }\r
index d6754ac..617d5fe 100644 (file)
@@ -4,7 +4,7 @@ if (window.testRunner)
     testRunner.dumpAsText();
 
 function runTest() {
-    window.getSelection().setBaseAndExtent(start, 0, null, 0);
+    window.getSelection().setBaseAndExtent(start, 0, start, 0);
     document.execCommand("Indent");
     document.body.innerHTML = "PASS. WebKit didn't crash.";
 }
index 6fad971..dd4ee1f 100644 (file)
@@ -3,7 +3,7 @@ if (window.testRunner)
     testRunner.dumpAsText();
 
 function runTest() {
-    window.getSelection().setBaseAndExtent(start, 0, null, 0);
+    window.getSelection().setBaseAndExtent(start, 0, start, 0);
     document.execCommand("Indent");
 
     document.writeln('execCommand("Indent") was crashing if the top element to be formatted is actually not an element.<br>');
index 5f1a137..706a6a8 100644 (file)
@@ -102,7 +102,7 @@ function setSelection(node)
     var textNode = node.firstChild;
     if (textNode.nodeType != Node.TEXT_NODE)
         throw "Wrong node type: " + textNode;
-    execSetSelectionCommand(textNode, 0, null);
+    execSetSelectionCommand(textNode, 0, textNode);
 }
 
 function verifyTextSelection(startNode, startOffset, endNode, endOffset)
index bdea94f..059f69e 100644 (file)
@@ -30,7 +30,7 @@ function setSelection(node)
     var textNode = node.firstChild;
     if (textNode.nodeType != Node.TEXT_NODE)
         throw "Wrong node type: " + textNode;
-    execSetSelectionCommand(textNode, 0, null);
+    execSetSelectionCommand(textNode, 0, textNode);
 }
 
 function verifyTextSelection(startNode, startOffset, endNode, endOffset)
index fca4c97..b7261a9 100644 (file)
@@ -63,7 +63,7 @@ else {
     Markup.dump(element, 'Overwrite behaves like insert on range selections');
 
     element.innerHTML = "&#x4E20;&#x4E21;&#x4E22;&#x4E23;";
-    selection.collapse();
+    selection.collapse(null);
     Markup.dump(element, 'New CJK contents');
 
     selection.collapse(element, 0);
@@ -71,7 +71,7 @@ else {
     Markup.dump(element, 'Overwrite CJK text');
 
     element.innerHTML="<div id=\"rtl-div\" dir=\"rtl\">&aleph;&beth;&gimel;&daleth;</div>"
-    selection.collapse();
+    selection.collapse(null);
     Markup.dump(element, 'New RTL contents');
 
     selection.collapse(element, 0);
index 5657793..5a7b687 100644 (file)
@@ -24,5 +24,5 @@ function dragSelectionToTarget(startSelectionElement, targetElement) {
     eventSender.mouseMoveTo(targetx, targety);
     eventSender.mouseUp();
 
-    window.getSelection().collapse();
+    window.getSelection().collapse(null);
 }
diff --git a/LayoutTests/editing/selection/bad-input-expected.txt b/LayoutTests/editing/selection/bad-input-expected.txt
new file mode 100644 (file)
index 0000000..b90786b
--- /dev/null
@@ -0,0 +1,28 @@
+Tests passing bad input to the Selection API.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+* Not passing enough parameters
+PASS selection.collapse() threw exception TypeError: Not enough arguments.
+PASS selection.containsNode() threw exception TypeError: Not enough arguments.
+PASS selection.selectAllChildren() threw exception TypeError: Not enough arguments.
+PASS selection.extend() threw exception TypeError: Not enough arguments.
+PASS selection.getRangeAt() threw exception TypeError: Not enough arguments.
+PASS selection.addRange() threw exception TypeError: Not enough arguments.
+PASS selection.setBaseAndExtent() threw exception TypeError: Not enough arguments.
+PASS selection.setBaseAndExtent(document.body) threw exception TypeError: Not enough arguments.
+PASS selection.setBaseAndExtent(document.body, 0) threw exception TypeError: Not enough arguments.
+PASS selection.setBaseAndExtent(document.body, 0, document.body) threw exception TypeError: Not enough arguments.
+PASS selection.setPosition() threw exception TypeError: Not enough arguments.
+
+* Passing null for non-nullable parameters
+PASS selection.containsNode(null) threw exception TypeError: Argument 1 ('node') to Selection.containsNode must be an instance of Node.
+PASS selection.selectAllChildren(null) threw exception TypeError: Argument 1 ('node') to Selection.selectAllChildren must be an instance of Node.
+PASS selection.extend(null) threw exception TypeError: Argument 1 ('node') to Selection.extend must be an instance of Node.
+PASS selection.addRange(null) threw exception TypeError: Argument 1 ('range') to Selection.addRange must be an instance of Range.
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/bad-input.html b/LayoutTests/editing/selection/bad-input.html
new file mode 100644 (file)
index 0000000..433fcc5
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Tests passing bad input to the Selection API.");
+
+var selection = getSelection();
+
+debug("* Not passing enough parameters");
+shouldThrowErrorName("selection.collapse()", "TypeError");
+shouldThrowErrorName("selection.containsNode()", "TypeError");
+shouldThrowErrorName("selection.selectAllChildren()", "TypeError");
+shouldThrowErrorName("selection.extend()", "TypeError");
+shouldThrowErrorName("selection.getRangeAt()", "TypeError");
+shouldThrowErrorName("selection.addRange()", "TypeError");
+shouldThrowErrorName("selection.setBaseAndExtent()", "TypeError");
+shouldThrowErrorName("selection.setBaseAndExtent(document.body)", "TypeError");
+shouldThrowErrorName("selection.setBaseAndExtent(document.body, 0)", "TypeError");
+shouldThrowErrorName("selection.setBaseAndExtent(document.body, 0, document.body)", "TypeError");
+shouldThrowErrorName("selection.setPosition()", "TypeError");
+
+debug("");
+debug("* Passing null for non-nullable parameters");
+shouldThrowErrorName("selection.containsNode(null)", "TypeError");
+shouldThrowErrorName("selection.selectAllChildren(null)", "TypeError");
+shouldThrowErrorName("selection.extend(null)", "TypeError");
+shouldThrowErrorName("selection.addRange(null)", "TypeError");
+debug("");
+</script> 
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 9a2164a..aaec6b2 100644 (file)
@@ -1,6 +1,4 @@
 foobarbaz
-Success: s.containsNode(null, false) is false.
-Success: s.containsNode(null, true) is false.
 Success: s.containsNode(testDiv, false) is false.
 Success: s.containsNode(testDiv, true) is true.
 Success: s.containsNode(span1, false) is false.
index 3c16b67..c3bf56d 100644 (file)
@@ -33,8 +33,6 @@ r.setStart(span1, 1);
 r.setEnd(span1, 2);
 s.addRange(r);
 
-shouldBe('s.containsNode(null, false)', false);
-shouldBe('s.containsNode(null, true)', false);
 shouldBe('s.containsNode(testDiv, false)', false);
 shouldBe('s.containsNode(testDiv, true)', true);
 shouldBe('s.containsNode(span1, false)', false);
index 4876bef..d1a96e7 100644 (file)
@@ -8,7 +8,7 @@
                 testRunner.dumpAsText();
 
             var target = document.getElementById("target");
-            getSelection().setBaseAndExtent(target.firstChild, 0, target.frstChild, 0);
+            getSelection().setBaseAndExtent(target.firstChild, 0, target.firstChild, 0);
             moveSelectionForwardByLineCommand();
 
             document.getElementById("result").innerText = getSelection().baseOffset == 63 ? "PASS" : "FAIL";
index 9188e17..76f9caf 100644 (file)
@@ -3,10 +3,10 @@ description("Test to check if setBaseAndExtent guard node with null owner docume
 var sel = window.getSelection();
 var docType = document.implementation.createDocumentType('c', '', '');
 
-sel.setBaseAndExtent(docType);
+sel.setBaseAndExtent(docType, 0, docType, 0);
 shouldBeNull("sel.anchorNode");
 
-sel.setBaseAndExtent(null, 0, docType, 0);
+sel.setBaseAndExtent(docType, 0, docType, 0);
 shouldBeNull("sel.anchorNode");
 
 sel.collapse(docType);
index 48eea43..0846e2e 100644 (file)
@@ -22,8 +22,8 @@ var mainSel = window.getSelection();
 
 function clear()
 {
-    foreignSel.setBaseAndExtent(null, 0, null, 0);
-    mainSel.setBaseAndExtent(null, 0, null, 0);
+    foreignSel.removeAllRanges();
+    mainSel.removeAllRanges();
 }
 
 mainSel.setBaseAndExtent(foreignElement, 0, foreignElement, 0);
index 0a5d32a..8bfc1e9 100644 (file)
@@ -64,7 +64,7 @@
         el7.setAttribute('id','el7')
         document.body.appendChild(el7)
         document.designMode='on'
-        window.getSelection().setBaseAndExtent(el3, 1)
+        window.getSelection().setBaseAndExtent(el3, 1, el3, 1)
         document.execCommand('InsertLineBreak')
         document.execCommand('selectall')
         document.execCommand('strikethrough')
index a659edf..c634a48 100644 (file)
@@ -58,10 +58,10 @@ PASS nonNumericPolicy('document.createRange().comparePoint(document, x)') is 'an
 PASS nonNumericPolicy('document.createRange().isPointInRange(document, x)') is 'any type allowed (but not omitted)'
 PASS nonNumericPolicy('getSelection().collapse(document, x)') is 'any type allowed'
 PASS nonNumericPolicy('getSelection().setBaseAndExtent(document, x, document, 0)') is 'any type allowed'
-PASS nonNumericPolicy('getSelection().setBaseAndExtent(document, 0, document, x)') is 'any type allowed'
+PASS nonNumericPolicy('getSelection().setBaseAndExtent(document, 0, document, x)') is 'any type allowed (but not omitted)'
 PASS nonNumericPolicy('getSelection().setPosition(document, x)') is 'any type allowed'
 PASS nonNumericPolicy('getSelection().extend(document, x)') is 'any type allowed'
-PASS nonNumericPolicy('getSelection().getRangeAt(x)') is 'any type allowed'
+PASS nonNumericPolicy('getSelection().getRangeAt(x)') is 'any type allowed (but not omitted)'
 PASS nonNumericPolicy('document.styleSheets.item(x)') is 'any type allowed (but not omitted)'
 PASS nonNumericPolicy('document.createTextNode("a").splitText(x)') is 'any type allowed (but not omitted)'
 PASS nonNumericPolicy('document.createTreeWalker(document, x, null, false)') is 'any type allowed'
index 8b49e7f..61522ea 100644 (file)
@@ -312,10 +312,10 @@ shouldBe("nonNumericPolicy('document.createRange().isPointInRange(document, x)')
 
 shouldBe("nonNumericPolicy('getSelection().collapse(document, x)')", "'any type allowed'");
 shouldBe("nonNumericPolicy('getSelection().setBaseAndExtent(document, x, document, 0)')", "'any type allowed'");
-shouldBe("nonNumericPolicy('getSelection().setBaseAndExtent(document, 0, document, x)')", "'any type allowed'");
+shouldBe("nonNumericPolicy('getSelection().setBaseAndExtent(document, 0, document, x)')", "'any type allowed (but not omitted)'");
 shouldBe("nonNumericPolicy('getSelection().setPosition(document, x)')", "'any type allowed'");
 shouldBe("nonNumericPolicy('getSelection().extend(document, x)')", "'any type allowed'");
-shouldBe("nonNumericPolicy('getSelection().getRangeAt(x)')", "'any type allowed'");
+shouldBe("nonNumericPolicy('getSelection().getRangeAt(x)')", "'any type allowed (but not omitted)'");
 
 // SQLResultSetRowList
 
index 775b138..e4a3e56 100644 (file)
@@ -39,7 +39,7 @@ if (window.testRunner) {
 
     // On Mac, home/end doesn't move caret so manually select " World".
     if (navigator.platform.indexOf('Mac') == 0)
-        window.getSelection().setBaseAndExtent(div.firstChild, div.textContent.indexOf('World'));
+        window.getSelection().setBaseAndExtent(div.firstChild, div.textContent.indexOf('World'), div.firstChild, div.textContent.indexOf('World') + 5);
 
     eventSender.keyDown("leftArrow");
     logResult('Check (Left arrow)', 1);
index 733900e..afd0a20 100644 (file)
@@ -50,7 +50,7 @@ var ed = document.getElementById("editable");
 var selection = window.getSelection();
 selection.selectAllChildren(ed);
 document.execCommand("FormatBlock", false, "nav");
-selection.collapse();
+selection.collapse(null);
 ed.blur();
 
 var tests = document.getElementById("testArea");
index 42e070e..eb33995 100644 (file)
@@ -35,7 +35,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'article');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"ARTICLE"');
 document.body.removeChild(editable);
 
index 1119440..13af098 100644 (file)
@@ -35,7 +35,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'aside');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"ASIDE"');
 document.body.removeChild(editable);
 
index f289e40..e788c3c 100644 (file)
@@ -38,7 +38,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'footer');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"FOOTER"');
 document.body.removeChild(editable);
 
index 40be5b5..1a50113 100644 (file)
@@ -38,7 +38,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'header');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"HEADER"');
 document.body.removeChild(editable);
 
index 5b232df..2efd787 100644 (file)
@@ -36,7 +36,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'hgroup');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"HGROUP"');
 document.body.removeChild(editable);
 
index 6e467cf..a68c6d5 100644 (file)
@@ -35,7 +35,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'main');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"MAIN"');
 document.body.removeChild(editable);
 
index a57663f..9de162e 100644 (file)
@@ -35,7 +35,7 @@ editable.contentEditable = true;
 var selection = window.getSelection();
 selection.selectAllChildren(editable);
 document.execCommand('FormatBlock', false, 'section');
-selection.collapse();
+selection.collapse(null);
 shouldBe('document.getElementById("span2").parentNode.nodeName', '"SECTION"');
 document.body.removeChild(editable);
 
index e70a968..19e1a14 100644 (file)
@@ -14,7 +14,7 @@
     var s = window.getSelection();
     var p1 = document.getElementById("item1");
     var p2 = document.getElementById("item2");
-    s.setBaseAndExtent(p1, 0, p2);
+    s.setBaseAndExtent(p1, 0, p2, 0);
     document.execCommand("Indent");
 
     // This code doesn't do anything initially, but the code below creates an iframe
index 835aeb3..9bfa7fd 100644 (file)
@@ -16,7 +16,7 @@ onload = function() {
 
     input=document.createElement('input');
     textPath.parentNode.insertBefore(input, textPath);
-    window.getSelection().setBaseAndExtent(input, 4);
+    window.getSelection().setBaseAndExtent(input, 4, input, 4);
 
     document.designMode='on';
     document.execCommand('Transpose');
index c8ea3d4..75141c8 100644 (file)
@@ -14,7 +14,7 @@
       document.designMode='on';
       filterInFirstRoot = document.getElementById('filterInFirstRoot');
       useElement = document.getElementById('useElement');
-      window.getSelection().setBaseAndExtent(filterInFirstRoot, 0);
+      window.getSelection().setBaseAndExtent(filterInFirstRoot, 0, filterInFirstRoot, 0);
       document.execCommand('ForwardDelete');
       document.designMode='off';
     }
index 0b2b1b5..ddfb905 100644 (file)
@@ -1,5 +1,55 @@
 2016-08-08  Chris Dumez  <cdumez@apple.com>
 
+        Align Selection API with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=160663
+
+        Reviewed by Ryosuke Niwa.
+
+        Align Selection API with the specification:
+        - https://www.w3.org/TR/selection-api/#idl-def-Selection
+
+        In particular, the following changes were made:
+        - Mark parameters as non-nullable when they should be.
+        - Mark parameters as mandatory when they should be.
+        - Use "unsigned long" type for offsets instead of "long".
+
+        This aligns our behavior with Firefox and Chrome.
+
+        Note that the Node parameters to setBaseAndExtent() operation were kept
+        nullable, which does not match the specification. This is intentional
+        as I worry about compatibility risk, especially considering they are
+        still nullable in Chrome. Only Firefox marks them as non-nullable.
+
+        Test: editing/selection/bad-input.html
+
+        * dom/Position.h:
+        (WebCore::Position::LegacyEditingOffset::value):
+        (WebCore::Position::LegacyEditingOffset::LegacyEditingOffset):
+        (WebCore::createLegacyEditingPosition):
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::anchorOffset):
+        (WebCore::DOMSelection::focusOffset):
+        (WebCore::DOMSelection::baseOffset):
+        (WebCore::DOMSelection::extentOffset):
+        (WebCore::DOMSelection::rangeCount):
+        (WebCore::DOMSelection::collapse):
+        (WebCore::DOMSelection::setBaseAndExtent):
+        (WebCore::DOMSelection::setPosition):
+        (WebCore::DOMSelection::extend):
+        (WebCore::DOMSelection::getRangeAt):
+        (WebCore::DOMSelection::addRange):
+        (WebCore::DOMSelection::deleteFromDocument):
+        (WebCore::DOMSelection::containsNode):
+        (WebCore::DOMSelection::selectAllChildren):
+        (WebCore::DOMSelection::shadowAdjustedOffset):
+        (WebCore::DOMSelection::modify): Deleted.
+        (WebCore::DOMSelection::shadowAdjustedNode): Deleted.
+        (WebCore::DOMSelection::isValidForPosition): Deleted.
+        * page/DOMSelection.h:
+        * page/DOMSelection.idl:
+
+2016-08-08  Chris Dumez  <cdumez@apple.com>
+
         Regression(r204239): Caused flaky crashes under ~Database()
         https://bugs.webkit.org/show_bug.cgi?id=160665
         <rdar://problem/27748065>
index 58ad04d..2e1189e 100644 (file)
@@ -72,14 +72,16 @@ public:
     // For creating legacy editing positions: (Anchor type will be determined from editingIgnoresContent(node))
     class LegacyEditingOffset {
     public:
-        int value() const { return m_offset; }
+        unsigned value() const { return m_offset; }
 
     private:
-        explicit LegacyEditingOffset(int offset) : m_offset(offset) { }
+        explicit LegacyEditingOffset(unsigned offset)
+            : m_offset(offset)
+        { }
 
-        friend Position createLegacyEditingPosition(PassRefPtr<Node>, int offset);
+        friend Position createLegacyEditingPosition(PassRefPtr<Node>, unsigned offset);
 
-        int m_offset;
+        unsigned m_offset;
     };
     WEBCORE_EXPORT Position(PassRefPtr<Node> anchorNode, LegacyEditingOffset);
 
@@ -228,7 +230,7 @@ private:
     bool m_isLegacyEditingPosition : 1;
 };
 
-inline Position createLegacyEditingPosition(PassRefPtr<Node> node, int offset)
+inline Position createLegacyEditingPosition(PassRefPtr<Node> node, unsigned offset)
 {
     return Position(node, Position::LegacyEditingOffset(offset));
 }
index 917d5e5..9f0ff86 100644 (file)
@@ -100,7 +100,7 @@ Node* DOMSelection::anchorNode() const
     return shadowAdjustedNode(anchorPosition(visibleSelection()));
 }
 
-int DOMSelection::anchorOffset() const
+unsigned DOMSelection::anchorOffset() const
 {
     if (!m_frame)
         return 0;
@@ -116,7 +116,7 @@ Node* DOMSelection::focusNode() const
     return shadowAdjustedNode(focusPosition(visibleSelection()));
 }
 
-int DOMSelection::focusOffset() const
+unsigned DOMSelection::focusOffset() const
 {
     if (!m_frame)
         return 0;
@@ -132,7 +132,7 @@ Node* DOMSelection::baseNode() const
     return shadowAdjustedNode(basePosition(visibleSelection()));
 }
 
-int DOMSelection::baseOffset() const
+unsigned DOMSelection::baseOffset() const
 {
     if (!m_frame)
         return 0;
@@ -148,7 +148,7 @@ Node* DOMSelection::extentNode() const
     return shadowAdjustedNode(extentPosition(visibleSelection()));
 }
 
-int DOMSelection::extentOffset() const
+unsigned DOMSelection::extentOffset() const
 {
     if (!m_frame)
         return 0;
@@ -180,23 +180,18 @@ String DOMSelection::type() const
     return "Range";
 }
 
-int DOMSelection::rangeCount() const
+unsigned DOMSelection::rangeCount() const
 {
     if (!m_frame)
         return 0;
     return m_frame->selection().isNone() ? 0 : 1;
 }
 
-void DOMSelection::collapse(Node* node, int offset, ExceptionCode& ec)
+void DOMSelection::collapse(Node* node, unsigned offset)
 {
     if (!m_frame)
         return;
 
-    if (offset < 0) {
-        ec = INDEX_SIZE_ERR;
-        return;
-    }
-
     if (!isValidForPosition(node))
         return;
 
@@ -241,16 +236,11 @@ void DOMSelection::empty()
     m_frame->selection().clear();
 }
 
-void DOMSelection::setBaseAndExtent(Node* baseNode, int baseOffset, Node* extentNode, int extentOffset, ExceptionCode& ec)
+void DOMSelection::setBaseAndExtent(Node* baseNode, unsigned baseOffset, Node* extentNode, unsigned extentOffset)
 {
     if (!m_frame)
         return;
 
-    if (baseOffset < 0 || extentOffset < 0) {
-        ec = INDEX_SIZE_ERR;
-        return;
-    }
-
     if (!isValidForPosition(baseNode) || !isValidForPosition(extentNode))
         return;
 
@@ -258,14 +248,10 @@ void DOMSelection::setBaseAndExtent(Node* baseNode, int baseOffset, Node* extent
     m_frame->selection().moveTo(createLegacyEditingPosition(baseNode, baseOffset), createLegacyEditingPosition(extentNode, extentOffset), DOWNSTREAM);
 }
 
-void DOMSelection::setPosition(Node* node, int offset, ExceptionCode& ec)
+void DOMSelection::setPosition(Node* node, unsigned offset)
 {
     if (!m_frame)
         return;
-    if (offset < 0) {
-        ec = INDEX_SIZE_ERR;
-        return;
-    }
 
     if (!isValidForPosition(node))
         return;
@@ -324,12 +310,12 @@ void DOMSelection::modify(const String& alterString, const String& directionStri
     m_frame->selection().modify(alter, direction, granularity);
 }
 
-void DOMSelection::extend(Node& node, int offset, ExceptionCode& ec)
+void DOMSelection::extend(Node& node, unsigned offset, ExceptionCode& ec)
 {
     if (!m_frame)
         return;
 
-    if (offset < 0 || offset > (node.offsetInCharacters() ? caretMaxOffset(node) : static_cast<int>(node.countChildNodes()))) {
+    if (offset > (node.offsetInCharacters() ? caretMaxOffset(node) : node.countChildNodes())) {
         ec = INDEX_SIZE_ERR;
         return;
     }
@@ -341,12 +327,12 @@ void DOMSelection::extend(Node& node, int offset, ExceptionCode& ec)
     m_frame->selection().setExtent(createLegacyEditingPosition(&node, offset), DOWNSTREAM);
 }
 
-RefPtr<Range> DOMSelection::getRangeAt(int index, ExceptionCode& ec)
+RefPtr<Range> DOMSelection::getRangeAt(unsigned index, ExceptionCode& ec)
 {
     if (!m_frame)
         return nullptr;
 
-    if (index < 0 || index >= rangeCount()) {
+    if (index >= rangeCount()) {
         ec = INDEX_SIZE_ERR;
         return nullptr;
     }
@@ -370,45 +356,42 @@ void DOMSelection::removeAllRanges()
     m_frame->selection().clear();
 }
 
-void DOMSelection::addRange(Range* r)
+void DOMSelection::addRange(Range& range)
 {
     if (!m_frame)
         return;
-    if (!r)
-        return;
 
     FrameSelection& selection = m_frame->selection();
-
     if (selection.isNone()) {
-        selection.moveTo(r);
+        selection.moveTo(&range);
         return;
     }
 
-    RefPtr<Range> range = selection.selection().toNormalizedRange();
-    if (!range)
+    RefPtr<Range> normalizedRange = selection.selection().toNormalizedRange();
+    if (!normalizedRange)
         return;
 
-    if (r->compareBoundaryPoints(Range::START_TO_START, *range, IGNORE_EXCEPTION) == -1) {
+    if (range.compareBoundaryPoints(Range::START_TO_START, *normalizedRange, IGNORE_EXCEPTION) == -1) {
         // We don't support discontiguous selection. We don't do anything if r and range don't intersect.
-        if (r->compareBoundaryPoints(Range::START_TO_END, *range, IGNORE_EXCEPTION) > -1) {
-            if (r->compareBoundaryPoints(Range::END_TO_END, *range, IGNORE_EXCEPTION) == -1) {
+        if (range.compareBoundaryPoints(Range::START_TO_END, *normalizedRange, IGNORE_EXCEPTION) > -1) {
+            if (range.compareBoundaryPoints(Range::END_TO_END, *normalizedRange, IGNORE_EXCEPTION) == -1) {
                 // The original range and r intersect.
-                selection.moveTo(r->startPosition(), range->endPosition(), DOWNSTREAM);
+                selection.moveTo(range.startPosition(), normalizedRange->endPosition(), DOWNSTREAM);
             } else {
                 // r contains the original range.
-                selection.moveTo(r);
+                selection.moveTo(&range);
             }
         }
     } else {
         // We don't support discontiguous selection. We don't do anything if r and range don't intersect.
         ExceptionCode ec = 0;
-        if (r->compareBoundaryPoints(Range::END_TO_START, *range, ec) < 1 && !ec) {
-            if (r->compareBoundaryPoints(Range::END_TO_END, *range, IGNORE_EXCEPTION) == -1) {
+        if (range.compareBoundaryPoints(Range::END_TO_START, *normalizedRange, ec) < 1 && !ec) {
+            if (range.compareBoundaryPoints(Range::END_TO_END, *normalizedRange, IGNORE_EXCEPTION) == -1) {
                 // The original range contains r.
-                selection.moveTo(range.get());
+                selection.moveTo(normalizedRange.get());
             } else {
                 // The original range and r intersect.
-                selection.moveTo(range->startPosition(), r->endPosition(), DOWNSTREAM);
+                selection.moveTo(normalizedRange->startPosition(), range.endPosition(), DOWNSTREAM);
             }
         }
     }
@@ -430,26 +413,26 @@ void DOMSelection::deleteFromDocument()
 
     selectedRange->deleteContents(ASSERT_NO_EXCEPTION);
 
-    setBaseAndExtent(&selectedRange->startContainer(), selectedRange->startOffset(), &selectedRange->startContainer(), selectedRange->startOffset(), ASSERT_NO_EXCEPTION);
+    setBaseAndExtent(&selectedRange->startContainer(), selectedRange->startOffset(), &selectedRange->startContainer(), selectedRange->startOffset());
 }
 
-bool DOMSelection::containsNode(Node* n, bool allowPartial) const
+bool DOMSelection::containsNode(Node& node, bool allowPartial) const
 {
     if (!m_frame)
         return false;
 
     FrameSelection& selection = m_frame->selection();
 
-    if (!n || m_frame->document() != &n->document() || selection.isNone())
+    if (m_frame->document() != &node.document() || selection.isNone())
         return false;
 
-    RefPtr<Node> node = n;
+    Ref<Node> protectedNode(node);
     RefPtr<Range> selectedRange = selection.selection().toNormalizedRange();
 
-    ContainerNode* parentNode = node->parentNode();
+    ContainerNode* parentNode = node.parentNode();
     if (!parentNode || !parentNode->inDocument())
         return false;
-    unsigned nodeIndex = node->computeNodeIndex();
+    unsigned nodeIndex = node.computeNodeIndex();
 
     ExceptionCode ec = 0;
     bool nodeFullySelected = Range::compareBoundaryPoints(parentNode, nodeIndex, &selectedRange->startContainer(), selectedRange->startOffset(), ec) >= 0 && !ec
@@ -464,16 +447,13 @@ bool DOMSelection::containsNode(Node* n, bool allowPartial) const
     if (nodeFullyUnselected)
         return false;
 
-    return allowPartial || node->isTextNode();
+    return allowPartial || node.isTextNode();
 }
 
-void DOMSelection::selectAllChildren(Node* n, ExceptionCode& ec)
+void DOMSelection::selectAllChildren(Node& node)
 {
-    if (!n)
-        return;
-
     // This doesn't (and shouldn't) select text node characters.
-    setBaseAndExtent(n, 0, n, n->countChildNodes(), ec);
+    setBaseAndExtent(&node, 0, &node, node.countChildNodes());
 }
 
 String DOMSelection::toString()
@@ -501,7 +481,7 @@ Node* DOMSelection::shadowAdjustedNode(const Position& position) const
     return adjustedNode->parentNodeGuaranteedHostFree();
 }
 
-int DOMSelection::shadowAdjustedOffset(const Position& position) const
+unsigned DOMSelection::shadowAdjustedOffset(const Position& position) const
 {
     if (position.isNull())
         return 0;
index 109505b..2d11e89 100644 (file)
@@ -57,11 +57,11 @@ namespace WebCore {
         // These methods return the valid equivalents of internal editing positions.
         Node* baseNode() const;
         Node* extentNode() const;
-        int baseOffset() const;
-        int extentOffset() const;
+        unsigned baseOffset() const;
+        unsigned extentOffset() const;
         String type() const;
-        void setBaseAndExtent(Node* baseNode, int baseOffset, Node* extentNode, int extentOffset, ExceptionCode&);
-        void setPosition(Node*, int offset, ExceptionCode&);
+        void setBaseAndExtent(Node* baseNode, unsigned baseOffset, Node* extentNode, unsigned extentOffset);
+        void setPosition(Node*, unsigned offset);
         void modify(const String& alter, const String& direction, const String& granularity);
 
         // Mozilla Selection Object API
@@ -71,21 +71,21 @@ namespace WebCore {
         // expansion.
         // These methods return the valid equivalents of internal editing positions.
         Node* anchorNode() const;
-        int anchorOffset() const;
+        unsigned anchorOffset() const;
         Node* focusNode() const;
-        int focusOffset() const;
+        unsigned focusOffset() const;
         bool isCollapsed() const;
-        int rangeCount() const;
-        void collapse(Node*, int offset, ExceptionCode&);
+        unsigned rangeCount() const;
+        void collapse(Node*, unsigned offset);
         void collapseToEnd(ExceptionCode&);
         void collapseToStart(ExceptionCode&);
-        void extend(Node&, int offset, ExceptionCode&);
-        RefPtr<Range> getRangeAt(int, ExceptionCode&);
+        void extend(Node&, unsigned offset, ExceptionCode&);
+        RefPtr<Range> getRangeAt(unsigned, ExceptionCode&);
         void removeAllRanges();
-        void addRange(Range*);
+        void addRange(Range&);
         void deleteFromDocument();
-        bool containsNode(Node*, bool partlyContained) const;
-        void selectAllChildren(Node*, ExceptionCode&);
+        bool containsNode(Node&, bool partlyContained) const;
+        void selectAllChildren(Node&);
 
         String toString();
 
@@ -101,7 +101,7 @@ namespace WebCore {
         const VisibleSelection& visibleSelection() const;
 
         Node* shadowAdjustedNode(const Position&) const;
-        int shadowAdjustedOffset(const Position&) const;
+        unsigned shadowAdjustedOffset(const Position&) const;
 
         bool isValidForPosition(Node*) const;
     };
index ac22050..e1704c6 100644 (file)
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// This is based off of Mozilla's Selection interface
-// https://developer.mozilla.org/En/DOM/Selection
+// https://www.w3.org/TR/selection-api/#idl-def-Selection
 [
     GenerateIsReachable=ImplFrame,
     InterfaceName=Selection,
 ] interface DOMSelection {
-    readonly attribute Node anchorNode;
-    readonly attribute long anchorOffset;
-    readonly attribute Node focusNode;
-    readonly attribute long focusOffset;
+    readonly attribute Node? anchorNode;
+    readonly attribute unsigned long anchorOffset;
+    readonly attribute Node? focusNode;
+    readonly attribute unsigned long focusOffset;
 
     readonly attribute boolean isCollapsed;
-    readonly attribute long rangeCount;
+    readonly attribute unsigned long rangeCount;
 
-    [RaisesException] void collapse(optional Node? node = null, optional long index = 0);
+    void collapse(Node? node, optional unsigned long offset = 0);
     [RaisesException] void collapseToEnd();
     [RaisesException] void collapseToStart();
 
     void deleteFromDocument();
-    boolean containsNode(optional Node? node = null, optional boolean allowPartial = false);
-    [RaisesException] void selectAllChildren(optional Node? node = null);
+    boolean containsNode(Node node, optional boolean allowPartial = false);
+    void selectAllChildren(Node node);
 
-    [RaisesException] void extend(Node node, optional long offset = 0);
+    [RaisesException] void extend(Node node, optional unsigned long offset = 0);
 
-    [RaisesException] Range getRangeAt(optional long index = 0);
+    [RaisesException] Range getRangeAt(unsigned long index);
     void removeAllRanges();
-    void addRange(optional Range? range = null);
+    void addRange(Range range);
 
 #if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
     [NotEnumerable] DOMString toString();
 #endif
 
-    // WebKit extensions
-    readonly attribute Node baseNode;
-    readonly attribute long baseOffset;
-    readonly attribute Node extentNode;
-    readonly attribute long extentOffset;
-
-    // WebKit's "type" accessor returns "None", "Range" and "Caret"
-    // IE's type accessor returns "none", "text" and "control"
     readonly attribute DOMString type;
 
+    void setBaseAndExtent(Node? baseNode, unsigned long baseOffset, Node? extentNode, unsigned long extentOffset);
+    void setPosition(Node? node, optional unsigned long offset = 0);
+
+    void empty();
+
+    // FIXME: The following operation should be implemented.
+    // void removeRange(Range range);
+
+    // WebKit extensions.
+
     // FIXME: Using "undefined" as default parameter value is wrong.
     void modify(optional DOMString alter = "undefined", optional DOMString direction = "undefined", optional DOMString granularity = "undefined");
-    [RaisesException] void setBaseAndExtent(optional Node? baseNode = null, optional long baseOffset = 0, optional Node? extentNode = null, optional long extentOffset = 0);
-    [RaisesException] void setPosition(optional Node? node = null, optional long offset = 0);
 
-    // IE extension
-    // http://msdn.microsoft.com/en-us/library/ms535869(VS.85).aspx
-    void empty();
+    readonly attribute Node baseNode;
+    readonly attribute unsigned long baseOffset;
+    readonly attribute Node extentNode;
+    readonly attribute unsigned long extentOffset;
 };