Skip to content

Commit

Permalink
[Live Range Selection] Some tests fail due to ApplyStyleCommand not c…
Browse files Browse the repository at this point in the history
…anonicalizing ending selection

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

Reviewed by Darin Adler.

Explicitly canonicalize positions in ApplyStyleCommand::updateStartEnd so that enabling live range selection
do not cause a new test failure. Also add variants of the tests that used to fail without this patch.

* LayoutTests/editing/execCommand/remove-format-background-color-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-format-background-color-live-range.html: Added.
* LayoutTests/editing/execCommand/remove-format-image-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-format-image-live-range.html: Added.
* LayoutTests/editing/execCommand/remove-format-in-table-cell-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-format-in-table-cell-live-range.html: Added.
* LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe-live-range.html: Added.
* LayoutTests/editing/execCommand/remove-formatting-2-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-formatting-2-live-range.html: Added.
* LayoutTests/editing/execCommand/remove-formatting-live-range-expected.txt: Added.
* LayoutTests/editing/execCommand/remove-formatting-live-range.html: Added.
* LayoutTests/editing/style/apply-font-size-to-multiple-nodes-live-range-expected.txt: Added.
* LayoutTests/editing/style/apply-font-size-to-multiple-nodes-live-range.html: Added.
* LayoutTests/editing/style/apply-style-atomic-live-range-expected.txt: Added.
* LayoutTests/editing/style/apply-style-atomic-live-range.html: Added.
* LayoutTests/editing/style/invalid-font-size-live-range-expected.txt: Added.
* LayoutTests/editing/style/invalid-font-size-live-range.html: Added.
* LayoutTests/editing/style/remove-styled-element-with-style-span-live-range-expected.txt: Added.
* LayoutTests/editing/style/remove-styled-element-with-style-span-live-range.html: Added.
* LayoutTests/editing/undo/redo-split-text-node-live-range-expected.txt: Added.
* LayoutTests/editing/undo/redo-split-text-node-live-range.html: Added.
* LayoutTests/platform/ios/editing/execCommand/remove-formatting-2-live-range-expected.txt: Added.

* Source/WebCore/editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::updateStartEnd):

Canonical link: https://commits.webkit.org/255409@main
  • Loading branch information
rniwa committed Oct 11, 2022
1 parent 28eb5d6 commit 75277ce
Show file tree
Hide file tree
Showing 24 changed files with 434 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
This tests removing format on text that has background color. There should be no span or inline style below:
| "<#selection-anchor>hello "
| "world"
| " WebKit.<#selection-focus>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<p id="description">This tests removing format on text that has background color. There should be no span or inline style below:</p>
<div id="editor" contenteditable>hello <span style="color: black; background-color: #ff0000;">world</span> WebKit.</div>
<script type="text/javascript" src="../../resources/dump-as-markup.js"></script>
<script type="text/javascript">

if (window.testRunner)
testRunner.dumpAsText();

var editor = document.getElementById('editor');
editor.focus();
document.execCommand('SelectAll', false, null);
document.execCommand('RemoveFormat', false, null);

Markup.description(document.getElementById('description').textContent);
Markup.dump(editor);

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
This tests RemoveFormant command not removing an image. You should see one image element in each of the test results

An image between text:
| "<#selection-anchor>hello"
| " "
| <img>
| src="../resources/abe.png"
| " world<#selection-focus>"

RemoveFormat on an image followed by JustifyNone and FormatBlock with p.:
| <p>
| <#selection-anchor>
| <img>
| src="../resources/abe.png"
| <#selection-focus>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body onload="runTests();">
<script src="../../resources/dump-as-markup.js"></script>
<div id="test1" contenteditable><strong>hello</strong> <img src="../resources/abe.png"> world</div>
<div id="test2" contenteditable><img src="../resources/abe.png"></div>
<script>

function runTests() {
Markup.description('This tests RemoveFormant command not removing an image. You should see one image element in each of the test results');
window.getSelection().selectAllChildren(document.getElementById('test1'));
document.execCommand('RemoveFormat', false, null);
Markup.dump('test1', 'An image between text');

window.getSelection().selectAllChildren(document.getElementById('test2'));
document.execCommand('RemoveFormat', false, null);
document.execCommand('JustifyNone', false, null);
document.execCommand('FormatBlock', false, 'p');
Markup.dump('test2', 'RemoveFormat on an image followed by JustifyNone and FormatBlock with p.');

Markup.notifyDone();
}

Markup.waitUntilDone();

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
This tests that RemoveFormat does not add extra span tags when executed inside a table cell.
| <table>
| <tbody>
| <tr>
| <td>
| "<#selection-anchor>hello<#selection-focus>"
| <tr>
| <td>
| "world"
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<script src="../../resources/dump-as-markup.js"></script>
<div id="test" contenteditable><table><tr><td><b>hello</b></td></tr><tr><td>world</td></tr></table></div>
<script>

Markup.description('This tests that RemoveFormat does not add extra span tags when executed inside a table cell.');
window.getSelection().selectAllChildren(document.getElementsByTagName('b')[0]);
document.execCommand('RemoveFormat', false, null);
Markup.dump('test');

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
This testcase is to test crash scenario when designMode is set on document and RemoveFormat is called. Expected result is that crash should not happen and underline should be removed from all the selected text
| "<#selection-anchor>This Test should not crash.\n "
| <iframe>
| onload="selectAndRemoveFormat()"
| <#selection-focus>
| "\n "
| <p>
| "PASS"
| "\n "

FRAME 0:
| <head>
| <body>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<script src="../../resources/dump-as-markup.js"></script>
<script>
function selectAndRemoveFormat()
{
document.designMode = 'on';
document.execCommand('SelectAll');
document.execCommand('RemoveFormat');
}

function dumpAfterCommand()
{
Markup.description('This testcase is to test crash scenario when designMode is set on document and RemoveFormat is called. Expected result is that crash should not happen and underline should be removed from all the selected text')
Markup.dump('container');
}
</script>
<body>
<div id="container" style="text-decoration: underline !important;">This Test should not crash.
<iframe onload="selectAndRemoveFormat()"></iframe>
<p>PASS</p>
</div>
<script>
dumpAfterCommand();
</script>
</body>
</html>

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 2 of DIV > BODY > HTML > #document
EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 4 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 4 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
This tests that RemoveFormat not only removes style from the selected part of the DOM, but that it also applies the document default style to the selection if that's necessary in order to leave the selected text unstyled.
| "<#selection-anchor>This<#selection-focus>"
| " text should look the same as the text above."
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<script src="../../resources/dump-as-markup.js"></script>
<div id="div" contenteditable="true"><b>This</b> text should look the same as the text above.</div>
<script>

if (window.testRunner)
testRunner.dumpEditingCallbacks();

if (window.internals)
internals.settings.setEditingBehavior('mac');

var sel = window.getSelection();
var div = document.getElementById("div");

sel.setPosition(div, 0);

sel.modify("extend", "forward", "word");
document.execCommand("RemoveFormat");

Markup.description("This tests that RemoveFormat not only removes style from the selected part of the DOM, but that it also applies the document default style to the selection if that's necessary in order to leave the selected text unstyled.");
Markup.dump(div);

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 13 of DIV > BODY > HTML > #document
EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > B > DIV > BODY > HTML > #document to 3 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > A > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
This is a test for execCommand("RemoveFormat"). It demonstrates a bug: everything in the editable region below should be selected, as everything was selected before Remove Format was performed.

markup:
| "\n"
| "<#selection-anchor>foo"
| <a>
| href="http://www.google.com/"
| "bar"
| "baz"
| <br>
| "\n"
| <table>
| border="1"
| <tbody>
| <tr>
| <td>
| "foo"
| <td>
| "bar"
| <td>
| "baz"
| "\n"
| "foo"
| "bar"
| "baz<#selection-focus>"
| <br>
| "\n"

console:
|
41 changes: 41 additions & 0 deletions LayoutTests/editing/execCommand/remove-formatting-live-range.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<script src="../../resources/dump-as-markup.js"></script>
<div id="test" contenteditable="true">
<b>foo</b><a href="http://www.google.com/">bar</a><i>baz</i><br>
<table border="1"><tr><td>foo</td><td>bar</td><td>baz</td></tr></table>
<u>foo</u>bar<span style="text-decoration:line-through">baz</span><br>
</div>
<pre id="console"></pre>
<script>

if (window.testRunner)
testRunner.dumpEditingCallbacks();

function log(message) {
var console = document.getElementById("console");
console.innerHTML += message + '\n';
}

var e = document.getElementById("test");
var s = window.getSelection();

if (document.queryCommandEnabled("RemoveFormat"))
log("Failure: RemoveFormat enabled with no selection.");
s.setPosition(e, 0);
if (document.queryCommandEnabled("RemoveFormat"))
log("Failure: RemoveFormat enabled with a caret selection.");
document.execCommand("SelectAll");
if (!document.queryCommandEnabled("RemoveFormat"))
log("Failure: RemoveFormat disabled with an editable selection.");
if (!document.execCommand("RemoveFormat"))
log("Failure: execCommand('RemoveFormat') returned false.");

Markup.description('This is a test for execCommand("RemoveFormat"). It demonstrates a bug: everything in the editable region below should be selected, as everything was selected before Remove Format was performed.');
Markup.dump(e, 'markup');
Markup.dump('console', 'console');

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Test that WebKit does not add multiple span or font elements when applying font-size to multiple nodes.

You should see exactly one font element with size="7" and exactly one span:
| <font>
| size="7"
| "\n<#selection-anchor>A man with 2\n"
| <span>
| style="font-family: Courier New, Courier"
| "font faces"
| <br>
| "\nDeath comes in all\n"
| "font sizes<#selection-focus>"
| "\n"
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<head>
<script src="../../resources/dump-as-markup.js"></script>
</head>
<body>
<div id="test" contenteditable>
A man with 2
<span style="font-family: Courier New, Courier">font faces</span><br>
Death comes in all
<span style="font-size: large">font sizes</span>
</div>
<script>

var test = document.getElementById('test');
document.getSelection().selectAllChildren(test);
document.execCommand('fontSize', false, 7);
Markup.description('Test that WebKit does not add multiple span or font elements when applying font-size to multiple nodes.')
Markup.dump(test, 'You should see exactly one font element with size="7" and exactly one span');

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
Test that WebKit does not crash when we apply style to atomic elements and that the style is not applied inside atomic elements.
| <a>
| href="a"
| "<#selection-anchor>1"
| <progress>
| <a>
| style=""
| "2"
| <shadow:root>
| <div>
| pseudo="-webkit-progress-inner-element"
| shadow:pseudoId="-webkit-progress-inner-element"
| <div>
| pseudo="-webkit-progress-bar"
| shadow:pseudoId="-webkit-progress-bar"
| <div>
| pseudo="-webkit-progress-value"
| style="width: -100%;"
| shadow:pseudoId="-webkit-progress-value"
| <#selection-focus>
24 changes: 24 additions & 0 deletions LayoutTests/editing/style/apply-style-atomic-live-range.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<head>
<script src="../../resources/dump-as-markup.js"></script>
</head>
<body>
<div id="edit" contentEditable="true">1<progress><a style>2</a></progress></div>
<script>
Markup.description('Test that WebKit does not crash when we apply style to atomic elements ' +
'and that the style is not applied inside atomic elements.')

function select(node) {
var range = document.createRange();
range.selectNodeContents(node);
window.getSelection().addRange(range);
}

var edit = document.getElementById("edit");
select(edit);
document.execCommand("createlink", false, "a");
Markup.dump(edit);
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This tests applying font size to text inside a font element with an invalid size attribute.
WebKit should not crash and there should be exactly one font element with size="4"
| <font>
| size="4"
| "<#selection-anchor>hello<#selection-focus>"
17 changes: 17 additions & 0 deletions LayoutTests/editing/style/invalid-font-size-live-range.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<script src="../../resources/dump-as-markup.js"></script>
<div id="test" contenteditable><font size="x">hello</font></div>
<script>

Markup.description('This tests applying font size to text inside a font element with an invalid size attribute.\n' +
'WebKit should not crash and there should be exactly one font element with size="4"')
var test = document.getElementById('test');
window.getSelection().selectAllChildren(test);
document.execCommand('fontSize', false, '4');
Markup.dump(test);

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This test ensures WebKit removes implicitly styled elements even if they had class="Apple-style-span". There should be no span below.
| <i>
| "<#selection-anchor>hello "
| "world"
| " WebKit<#selection-focus>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html><!-- webkit-test-runner [ LiveRangeSelectionEnabled=true ] -->
<html>
<body>
<div id="test" contenteditable>hello <em class="Apple-style-span">world</em> WebKit</div>
<script src="../../resources/dump-as-markup.js"></script>
<script>

Markup.description('This test ensures WebKit removes implicitly styled elements even if they had class="Apple-style-span".'
+ ' There should be no span below.');

var test = document.getElementById('test');
test.focus();
document.execCommand('SelectAll', false, null);
document.execCommand('Italic', false, null);

Markup.dump(test);

</script>
</body>
</html>

0 comments on commit 75277ce

Please sign in to comment.