Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Selection API: Fix more editing tests that rely on non-standard selec…
…tion API behavior

https://bugs.webkit.org/show_bug.cgi?id=217301

Reviewed by Sam Weinig.

These changes do not change what the tests are testing. They remove
dependencies in these tests on "canonicalization" of selection points.
This is often always a change to the setup for the test, not the test substance.

In other cases, tests that are not trying to test selection are leaving unimportant
remnants of the selection in their results: clear the selection for those.

Patch also includes a few other expectations fixes so all the tests in the editing
directory pass for me on my development machine.

* editing/pasteboard/copy-null-characters-expected.txt: Remove expected selection.
* editing/pasteboard/copy-null-characters.html: Clear the selection after one part
of this test so the markup dump doesn't contain selection endpoints.

* editing/pasteboard/paste-noscript-xhtml-expected.txt: Remove expected selection.
* editing/pasteboard/paste-noscript-xhtml.xhtml: Clear the selection at two points
in this test so the markup dump doesn't contain selection endpoints.

* editing/selection/extend-by-sentence-002.html: Check the selected string rather
than checking the DOM node and offset of the selection. Keeps the substance of this
test without depending on the selection object selecting differently than what we
tell it to do.

* editing/selection/extend-selection-enclosing-block-mac.html: Set the position
at the start of a text node instead of before the text node.
* editing/selection/extend-selection-enclosing-block-win.html: Ditto.

* editing/selection/home-end.html: Added a setPositionAfterLeadingWhitespace
function and use it to set the position at the point the test results are
expecting. Before this change, the code relied on the Selection API to adjust the
selection to the first non-whitespace character, even though that is both non-standard
and not what we are testing. Doing this lets us keep the expected test results the same.

* editing/selection/move-by-word-visually-multi-line-expected.txt: Updated to
expect moving across the word, even the one that was preceded by a collapsed newline.
* editing/selection/move-by-word-visually-multi-line.html: Ditto.

* editing/selection/resources/extend-selection.js:
(setPositionAfterLeadingWhitespace): Added.
(runSelectionTestsWithGranularity): Use setPositionAfterLeadingWhitespace here
for the same purpose as in home-end.html above, to preserve the existing tests
without relying on non-standard Selection API behavior.

* editing/selection/resources/move-by-word-visually.js:
(setPositionAfterCollapsedLeadingWhitespace): Added.
(moveByWordForEveryPosition): Same as above, use the function.

* editing/selection/setBaseAndExtent-revert-selection.html: Fix the output function
to properly write out the node ID when the node is a text node within the node with
the ID. Before, the test could fail if the anchor node returned the text node we
passed in, but that relies on non-standard Selection API changes to the passed-in node.

* editing/selection/toString-1.html: Write out the unexpected result when the test
fails. It's not failing, but at one point it was, and this made it easier to diagnose.

* editing/undo/undo-after-event-edited.html: Use the Delete command to do a deletion
that we later undo. The old code was using the Selection API extractContents method,
which is not an undoable editing operation.

* platform/gtk/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/ios/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/mac-bigsur-wk1/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/mac-bigsur/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/win/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/wincairo/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
These unused files were incorrectly left behind when we converted this into a reftest.

* platform/mac-wk1/TestExpectations: Changed retro-correction-spelling-markers.html
to "[ Pass Failure ]" since it's a flaky test, not a reliable failure.

* platform/mac/TestExpectations: Changed some flakiness expectations that were marked
Release-only to be in effect in Debug builds too. For autocorrection-contraction.html,
expect a timeout on BigSur+ Debug. Not sure exactly what is causing it, but it's happening
both locally for me and on the bots.

* platform/wk2/TestExpectations: Expect a timeout in Debug builds for two tests that are
timing out both for me and on the Modern WebKit Debug bots, but not Legacy WebKit and not
Release bots.

Canonical link: https://commits.webkit.org/230100@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267997 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
darinadler committed Oct 5, 2020
1 parent 41aca86 commit 0c4c7fc
Show file tree
Hide file tree
Showing 25 changed files with 166 additions and 284 deletions.
86 changes: 86 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,89 @@
2020-10-04 Darin Adler <darin@apple.com>

Selection API: Fix more editing tests that rely on non-standard selection API behavior
https://bugs.webkit.org/show_bug.cgi?id=217301

Reviewed by Sam Weinig.

These changes do not change what the tests are testing. They remove
dependencies in these tests on "canonicalization" of selection points.
This is often always a change to the setup for the test, not the test substance.

In other cases, tests that are not trying to test selection are leaving unimportant
remnants of the selection in their results: clear the selection for those.

Patch also includes a few other expectations fixes so all the tests in the editing
directory pass for me on my development machine.

* editing/pasteboard/copy-null-characters-expected.txt: Remove expected selection.
* editing/pasteboard/copy-null-characters.html: Clear the selection after one part
of this test so the markup dump doesn't contain selection endpoints.

* editing/pasteboard/paste-noscript-xhtml-expected.txt: Remove expected selection.
* editing/pasteboard/paste-noscript-xhtml.xhtml: Clear the selection at two points
in this test so the markup dump doesn't contain selection endpoints.

* editing/selection/extend-by-sentence-002.html: Check the selected string rather
than checking the DOM node and offset of the selection. Keeps the substance of this
test without depending on the selection object selecting differently than what we
tell it to do.

* editing/selection/extend-selection-enclosing-block-mac.html: Set the position
at the start of a text node instead of before the text node.
* editing/selection/extend-selection-enclosing-block-win.html: Ditto.

* editing/selection/home-end.html: Added a setPositionAfterLeadingWhitespace
function and use it to set the position at the point the test results are
expecting. Before this change, the code relied on the Selection API to adjust the
selection to the first non-whitespace character, even though that is both non-standard
and not what we are testing. Doing this lets us keep the expected test results the same.

* editing/selection/move-by-word-visually-multi-line-expected.txt: Updated to
expect moving across the word, even the one that was preceded by a collapsed newline.
* editing/selection/move-by-word-visually-multi-line.html: Ditto.

* editing/selection/resources/extend-selection.js:
(setPositionAfterLeadingWhitespace): Added.
(runSelectionTestsWithGranularity): Use setPositionAfterLeadingWhitespace here
for the same purpose as in home-end.html above, to preserve the existing tests
without relying on non-standard Selection API behavior.

* editing/selection/resources/move-by-word-visually.js:
(setPositionAfterCollapsedLeadingWhitespace): Added.
(moveByWordForEveryPosition): Same as above, use the function.

* editing/selection/setBaseAndExtent-revert-selection.html: Fix the output function
to properly write out the node ID when the node is a text node within the node with
the ID. Before, the test could fail if the anchor node returned the text node we
passed in, but that relies on non-standard Selection API changes to the passed-in node.

* editing/selection/toString-1.html: Write out the unexpected result when the test
fails. It's not failing, but at one point it was, and this made it easier to diagnose.

* editing/undo/undo-after-event-edited.html: Use the Delete command to do a deletion
that we later undo. The old code was using the Selection API extractContents method,
which is not an undoable editing operation.

* platform/gtk/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/ios/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/mac-bigsur-wk1/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/mac-bigsur/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/win/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
* platform/wincairo/editing/pasteboard/drop-text-without-selection-expected.txt: Removed.
These unused files were incorrectly left behind when we converted this into a reftest.

* platform/mac-wk1/TestExpectations: Changed retro-correction-spelling-markers.html
to "[ Pass Failure ]" since it's a flaky test, not a reliable failure.

* platform/mac/TestExpectations: Changed some flakiness expectations that were marked
Release-only to be in effect in Debug builds too. For autocorrection-contraction.html,
expect a timeout on BigSur+ Debug. Not sure exactly what is causing it, but it's happening
both locally for me and on the bots.

* platform/wk2/TestExpectations: Expect a timeout in Debug builds for two tests that are
timing out both for me and on the Modern WebKit Debug bots, but not Legacy WebKit and not
Release bots.

2020-10-05 Rob Buis <rbuis@igalia.com>

WebKit doesn't parse "#" as delimiter for fragment identifier in data URIs
Expand Down
Expand Up @@ -32,7 +32,6 @@ If there are NULL characters in text nodes, they should not be copied to the cli
| "Copy paste me"
| "
"
| <#selection-caret>
| <textarea>
| id="destination-plain-text"
| this.value="Copy paste mebold
Expand Down
2 changes: 2 additions & 0 deletions LayoutTests/editing/pasteboard/copy-null-characters.html
Expand Up @@ -60,6 +60,8 @@
destinationPlainText.focus();
document.execCommand("Paste");

sel.empty();

var expectedPlainTextValue2 = "Copy paste mebold\n\nCopy paste me\ngreen";
if (expectedPlainTextValue2 != destinationPlainText.value) {
results.innerText = "Plain text field has the wrong value (expected " +
Expand Down
Expand Up @@ -13,7 +13,7 @@ Copied content:
| onclick="sayHello()"
| ondblclick="sayHello()"
| style="width: 100px;"
| "<#selection-anchor>Hello"
| "Hello"
| <a>
| href="http://www.bing.com/search?q=cnn"
| id="anchor1"
Expand Down Expand Up @@ -48,7 +48,7 @@ Copied content:
| "This is a form. "
| <button>
| formaction="javascript:sayHello()"
| "Submit.<#selection-focus>"
| "Submit."
| "

"
Expand Down Expand Up @@ -79,7 +79,7 @@ Pasted content:
| "This is a form."
| " "
| <button>
| "Submit.<#selection-caret>"
| "Submit."

FRAME 0:
| <head>
Expand Down
2 changes: 2 additions & 0 deletions LayoutTests/editing/pasteboard/paste-noscript-xhtml.xhtml
Expand Up @@ -20,11 +20,13 @@ function runTest() {
getSelection().setPosition(doc.body, 0);
doc.execCommand("SelectAll");
doc.execCommand("Copy");
frames[0].getSelection().empty();

Markup.dump(doc.body, 'Copied content');

getSelection().setPosition(document.getElementById("pastehere"), 0);
document.execCommand("Paste");
getSelection().empty();
Markup.dump('pastehere', 'Pasted content');

Markup.notifyDone();
Expand Down
7 changes: 3 additions & 4 deletions LayoutTests/editing/selection/extend-by-sentence-002.html
Expand Up @@ -9,12 +9,11 @@
var sel = getSelection();
sel.setBaseAndExtent(target, 0, target, 1);
sel.modify("extend", "forward", "sentence");
var selectedRange = sel.getRangeAt(0);
var selectedText = sel.toString().trim();

if (selectedRange.startContainer === target.firstChild && selectedRange.startOffset === 0
&& selectedRange.endContainer === target.nextSibling && selectedRange.endOffset === 0) {
if (selectedText == "All of this should be selected") {
result.innerText = "PASS";
} else {
result.innerText = "FAIL: Selected range is '" + selectedRange + "'";
result.innerText = "FAIL: Selected range is '" + selectedText + "'";
}
</script>
Expand Up @@ -14,7 +14,7 @@
function runSelectionTestsWithGranularityForEnclosingBlock(testNodes, granularity)
{
for (var i = 0; i < testNodes.length; ++i) {
getSelection().setPosition(testNodes[i], 0);
getSelection().setPosition(testNodes[i].firstChild, 0);

log("Test " + (i + 1) + ", LTR:\n");
log(" Extending right: ");
Expand Down
Expand Up @@ -11,7 +11,7 @@
function runSelectionTestsWithGranularityForEnclosingBlock(testNodes, granularity)
{
for (var i = 0; i < testNodes.length; ++i) {
getSelection().setPosition(testNodes[i], 0);
getSelection().setPosition(testNodes[i].firstChild, 0);

log("Test " + (i + 1) + ", LTR:\n");
log(" Extending right: ");
Expand Down
14 changes: 10 additions & 4 deletions LayoutTests/editing/selection/home-end.html
Expand Up @@ -62,6 +62,12 @@
log("]");
}

function setPositionAfterLeadingWhitespace(parentNode)
{
let textNode = parentNode.firstChild;
getSelection().setPosition(textNode, Math.max(textNode.data.search(/\S/), 0));
}

onload = function()
{
if (!window.testRunner)
Expand All @@ -77,7 +83,7 @@
var positionsMovingBackward;

log("Test " + (i + 1) + ", LTR:\n Moving forward: ");
sel.setPosition(tests[i], 0);
setPositionAfterLeadingWhitespace(tests[i]);
positionsMovingForward = positionsMovingInDirection(sel, "forward");
logPositions(positionsMovingForward);
log("\n");
Expand All @@ -90,7 +96,7 @@
tests[i].style.direction = "rtl";

log("Test " + (i + 1) + ", RTL:\n Moving forward: ");
sel.setPosition(tests[i], 0);
setPositionAfterLeadingWhitespace(tests[i]);
positionsMovingForward = positionsMovingInDirection(sel, "forward");
logPositions(positionsMovingForward);
log("\n");
Expand All @@ -109,7 +115,7 @@
tests[i].style.direction = "ltr";

log("Test " + (i + 1) + ", LTR:\n Moving right: ");
sel.setPosition(tests[i], 0);
setPositionAfterLeadingWhitespace(tests[i]);
positionsMovingRight = positionsMovingInDirection(sel, "right");
logPositions(positionsMovingRight);
log("\n");
Expand All @@ -122,7 +128,7 @@
tests[i].style.direction = "rtl";

log("Test " + (i + 1) + ", RTL:\n Moving left: ");
sel.setPosition(tests[i], 0);
setPositionAfterLeadingWhitespace(tests[i]);
positionsMovingLeft = positionsMovingInDirection(sel, "left");
logPositions(positionsMovingLeft);
log("\n");
Expand Down
Expand Up @@ -92,7 +92,7 @@ Move left by one word
<DIV>[0]
Test 19, LTR:
Move right by one word
"\n00"[3]
"\n00"[1, 3]
Move left by one word
"\n00"[3, 1]

Expand Up @@ -82,7 +82,7 @@
<div class="test_move_by_word" contenteditable dir=ltr title="0|0"></div>

<div contenteditable>
00<base dir=ltr class="test_move_by_word" title="3|3 1">
00<base dir=ltr class="test_move_by_word" title="1 3|3 1">
</div>

</div>
Expand Down
17 changes: 13 additions & 4 deletions LayoutTests/editing/selection/resources/extend-selection.js
Expand Up @@ -149,6 +149,15 @@ function extendAndLogSelectionToEnd(direction, granularity)
return extendAndLogSelection(extendSelectionToEnd, direction, granularity);
}

function setPositionAfterLeadingWhitespace(node)
{
let textNode = node.firstChild;
if (!textNode || textNode.nodeType != Node.TEXT_NODE)
getSelection().setPosition(node, 0);
else
getSelection().setPosition(textNode, Math.max(textNode.data.search(/\S/), 0));
}

function runSelectionTestsWithGranularity(testNodes, granularity)
{
for (var i = 0; i < testNodes.length; ++i) {
Expand All @@ -158,14 +167,14 @@ function runSelectionTestsWithGranularity(testNodes, granularity)
testNode.style.direction = "ltr";

log(" Extending right: ");
getSelection().setPosition(testNode);
setPositionAfterLeadingWhitespace(testNode);
var ltrRightPos = extendAndLogSelectionToEnd("right", granularity);

log(" Extending left: ");
var ltrLeftPos = extendAndLogSelectionToEnd("left", granularity);

log(" Extending forward: ");
getSelection().setPosition(testNode);
setPositionAfterLeadingWhitespace(testNode);
var ltrForwardPos = extendAndLogSelectionToEnd("forward", granularity);

log(" Extending backward: ");
Expand All @@ -175,14 +184,14 @@ function runSelectionTestsWithGranularity(testNodes, granularity)
testNode.style.direction = "rtl";

log(" Extending left: ");
getSelection().setPosition(testNode);
setPositionAfterLeadingWhitespace(testNode);
var rtlLeftPos = extendAndLogSelectionToEnd("left", granularity);

log(" Extending right: ");
var rtlRightPos = extendAndLogSelectionToEnd("right", granularity);

log(" Extending forward: ");
getSelection().setPosition(testNode);
setPositionAfterLeadingWhitespace(testNode);
var rtlForwardPos = extendAndLogSelectionToEnd("forward", granularity);

log(" Extending backward: ");
Expand Down
30 changes: 25 additions & 5 deletions LayoutTests/editing/selection/resources/move-by-word-visually.js
Expand Up @@ -224,15 +224,35 @@ function moveByWordOnEveryChar(sel, test, searchDirection, dir)
};
}

function setPositionAfterCollapsedLeadingWhitespace(node)
{
let textNode = node.firstChild;

// This goes beyond what the title of the function says. This strange special case keeps
// one of our tests working. It relies on selection being moved upstream out of the
// specified node; we could later just update the test to not rely on that strange thing.
if (!textNode && node.nodeType == Node.ELEMENT_NODE && node.localName == "base")
textNode = node.previousSibling;

if (!textNode || textNode.nodeType != Node.TEXT_NODE)
getSelection().setPosition(node, 0);
else {
let offset = 0;
if (getComputedStyle(node).getPropertyValue("white-space") != "pre")
offset = Math.max(textNode.data.search(/\S/), 0);
getSelection().setPosition(textNode, offset);
}
}

function moveByWordForEveryPosition(sel, test, dir)
{
// Check ctrl-right-arrow works for every position.
sel.setPosition(test, 0);
setPositionAfterCollapsedLeadingWhitespace(test);
var direction = "right";
if (dir == "rtl")
direction = "left";
moveByWord(sel, test, direction, dir);
sel.setPosition(test, 0);
direction = "left";
moveByWord(sel, test, direction, dir);
setPositionAfterCollapsedLeadingWhitespace(test);
moveByWordOnEveryChar(sel, test, direction, dir);

sel.modify("move", "forward", "lineBoundary");
Expand All @@ -243,7 +263,7 @@ function moveByWordForEveryPosition(sel, test, dir)
direction = "left";
else
direction = "right";
moveByWord(sel, test, direction, dir);
moveByWord(sel, test, direction, dir);

sel.setPosition(position.node, position.offset);
moveByWordOnEveryChar(sel, test, direction, dir);
Expand Down
Expand Up @@ -4,8 +4,11 @@
var output = '';

function selectNodes(a, b) {
window.getSelection().setBaseAndExtent(a, 1, b, 1);
output += window.getSelection().anchorNode.parentNode.id + ' ';
getSelection().setBaseAndExtent(a, 1, b, 1);
let node = getSelection().anchorNode;
if (node.nodeType == Node.TEXT_NODE)
node = node.parentNode;
output += node.id + ' ';
}

if (window.testRunner)
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/editing/selection/toString-1.html
Expand Up @@ -27,7 +27,7 @@
if (selection.toString) {
var string = selection.toString();
if (string != "\nbbbb")
throw("toString returned unexpected result");
throw("toString returned unexpected result: \"" + string.replace("\n", "\\n") + "\"");
log("success");
} else
throw("Selection::toString() not supported");
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/editing/undo/undo-after-event-edited.html
Expand Up @@ -35,7 +35,7 @@

window.addEventListener("load", function() {
document.execCommand("SelectAll", false)
var documentFragment = getSelection().getRangeAt(0).extractContents();
document.execCommand("Delete", false)
document.execCommand('Undo', false);
});

Expand Down

0 comments on commit 0c4c7fc

Please sign in to comment.