Diff
Modified: trunk/LayoutTests/ChangeLog (264417 => 264418)
--- trunk/LayoutTests/ChangeLog 2020-07-15 20:18:13 UTC (rev 264417)
+++ trunk/LayoutTests/ChangeLog 2020-07-15 20:28:06 UTC (rev 264418)
@@ -1,3 +1,24 @@
+2020-07-15 Daniel Bates <daba...@apple.com>
+
+ [iOS] Caret should be before text placeholder instead of after it
+ https://bugs.webkit.org/show_bug.cgi?id=214319
+ <rdar://problem/65295523>
+
+ Reviewed by Wenson Hsieh and Simon Fraser (1).
+
+ Add some tests.
+
+ [1] Simon only reviewed editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html.
+
+ * editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word-expected.txt: Added.
+ * editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word.html: Added.
+ * editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word-expected.txt: Added.
+ * editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word.html: Added.
+ * editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event-expected.txt: Added.
+ * editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html: Added.
+ * resources/js-test.js:
+ (shouldBeLessThanOrEqual): Added. Just like shouldBeGreaterThanOrEqual, but for <=.
+
2020-07-15 Simon Fraser <simon.fra...@apple.com>
itsnicethat.com page is sometimes non-scrollable
Added: trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word-expected.txt (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word-expected.txt 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,11 @@
+This tests that the text insertion point is before the inserted text placeholder when it's inserted at the end of a word. This test can only be run in WebKitTestRunner or DumpRenderTree.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getSelection().type is "Caret"
+PASS window.getSelection().baseOffset is 'Hello'.length
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word.html (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word.html (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word.html 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,47 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p id="description"></p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello from Cupertino!</div>
+<p id="console"></p>
+<script>
+window.jsTestIsAsync = true;
+
+description("This tests that the text insertion point is before the inserted text placeholder when it's inserted at the end of a word. This test can only be run in WebKitTestRunner or DumpRenderTree.");
+
+async function runTest()
+{
+ let testElement = document.getElementById("test");
+ if (window.testRunner)
+ await UIHelper.activateElementAndWaitForInputSession(testElement);
+ else
+ await UIHelper.callFunctionAndWaitForEvent(() => testElement.focus(), testElement, "focus");
+
+ let endOfHello = testElement.textContent.indexOf("Hello") + 5;
+ window.getSelection().setBaseAndExtent(testElement.firstChild, endOfHello, testElement.firstChild, endOfHello);
+
+ if (window.internals) {
+ let lineHeight = parseInt(window.getComputedStyle(testElement, null).lineHeight, 10);
+ internals.insertTextPlaceholder(0 /* width */, lineHeight);
+ }
+
+ shouldBeEqualToString("window.getSelection().type", "Caret");
+ shouldBe("window.getSelection().baseOffset", "'Hello'.length");
+
+ if (window.testRunner) {
+ // Cleanup to make the results pretty.
+ document.body.removeChild(testElement);
+ }
+
+ finishJSTest();
+}
+runTest();
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word-expected.txt (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word-expected.txt 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,11 @@
+This tests that the text insertion point is before the inserted text placeholder when it's inserted before the start of a word. This test can only be run in WebKitTestRunner or DumpRenderTree.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getSelection().type is "Caret"
+PASS window.getSelection().baseOffset is 'Hello'.length
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word.html (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word.html (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word.html 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,47 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p id="description"></p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello from Cupertino!</div>
+<p id="console"></p>
+<script>
+window.jsTestIsAsync = true;
+
+description("This tests that the text insertion point is before the inserted text placeholder when it's inserted before the start of a word. This test can only be run in WebKitTestRunner or DumpRenderTree.");
+
+async function runTest()
+{
+ let testElement = document.getElementById("test");
+ if (window.testRunner)
+ await UIHelper.activateElementAndWaitForInputSession(testElement);
+ else
+ await UIHelper.callFunctionAndWaitForEvent(() => testElement.focus(), testElement, "focus");
+
+ let positionOfFrom = testElement.textContent.indexOf("from");
+ window.getSelection().setBaseAndExtent(testElement.firstChild, positionOfFrom, testElement.firstChild, positionOfFrom);
+
+ if (window.internals) {
+ let lineHeight = parseInt(window.getComputedStyle(testElement, null).lineHeight, 10);
+ internals.insertTextPlaceholder(0 /* width */, lineHeight);
+ }
+
+ shouldBeEqualToString("window.getSelection().type", "Caret");
+ shouldBe("window.getSelection().baseOffset", "'Hello'.length");
+
+ if (window.testRunner) {
+ // Cleanup to make the results pretty.
+ document.body.removeChild(testElement);
+ }
+
+ finishJSTest();
+}
+runTest();
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event-expected.txt (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event-expected.txt 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,20 @@
+This tests that a text placeholder that is removed from the page via a mutation event callback after being inserted does not cause a crash. This test can only be run in WebKitTestRunner or DumpRenderTree.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Before inserting placeholder:
+PASS document.getElementById("test").childNodes.length is 1
+
+After inserting placeholder:
+PASS document.getElementById("test").childNodes.length is 3
+PASS placeholderElement.parentNode is non-null.
+
+After removing placeholder:
+PASS document.getElementById("test").childNodes.length became different from 3
+PASS document.getElementById("test").childNodes.length is <= 2
+PASS placeholderElement.parentNode became null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html (0 => 264418)
--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html 2020-07-15 20:28:06 UTC (rev 264418)
@@ -0,0 +1,77 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p id="description"></p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello from Cupertino!</div>
+<p id="console"></p>
+<script>
+window.jsTestIsAsync = true;
+
+description("This tests that a text placeholder that is removed from the page via a mutation event callback after being inserted does not cause a crash. This test can only be run in WebKitTestRunner or DumpRenderTree.");
+
+let placeholderElement;
+
+function done()
+{
+ // Clean up to make the result pretty.
+ if (window.testRunner)
+ document.getElementById("test").remove();
+ finishJSTest();
+}
+
+function didRemovePlaceholder()
+{
+ debug("<br>After removing placeholder:");
+ // Number of child nodes should be 2 or fewer. It depends on how aggressively adjacent #text nodes are coalesced.
+ shouldBecomeDifferent('document.getElementById("test").childNodes.length', "3", () => {
+ shouldBeLessThanOrEqual('document.getElementById("test").childNodes.length', "2");
+ shouldBecomeEqual("placeholderElement.parentNode", "null", done);
+ });
+}
+
+function didInsertPlaceholder()
+{
+ debug("<br>After inserting placeholder:");
+ shouldBe('document.getElementById("test").childNodes.length', "3");
+
+ console.assert(document.getElementById("test").childNodes.length >= 2);
+ console.assert(document.getElementById("test").childNodes[1] instanceof Element);
+
+ document.addEventListener("DOMSubtreeModified", didRemovePlaceholder, { once: true });
+ placeholderElement = document.getElementById("test").childNodes[1];
+ shouldBeNonNull("placeholderElement.parentNode");
+ placeholderElement.remove();
+}
+
+async function runTest()
+{
+ let testElement = document.getElementById("test");
+ if (window.testRunner)
+ await UIHelper.activateElementAndWaitForInputSession(testElement);
+ else
+ await UIHelper.callFunctionAndWaitForEvent(() => testElement.focus(), testElement, "focus");
+
+ let positionOfFrom = testElement.textContent.indexOf("from");
+ window.getSelection().setBaseAndExtent(testElement.firstChild, positionOfFrom, testElement.firstChild, positionOfFrom);
+
+ debug("Before inserting placeholder:")
+ shouldBeEqualToNumber('document.getElementById("test").childNodes.length', 1);
+
+ // To allow for the control flow to be debugged on Mac, add the event listener here instead of right before insertTextPlaceholder.
+ document.addEventListener("DOMSubtreeModified", didInsertPlaceholder, { once: true });
+
+ if (window.internals) {
+ let lineHeight = parseInt(window.getComputedStyle(testElement, null).lineHeight, 10);
+ internals.insertTextPlaceholder(0 /* width */, lineHeight);
+ }
+}
+runTest();
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/resources/js-test.js (264417 => 264418)
--- trunk/LayoutTests/resources/js-test.js 2020-07-15 20:18:13 UTC (rev 264417)
+++ trunk/LayoutTests/resources/js-test.js 2020-07-15 20:28:06 UTC (rev 264418)
@@ -620,6 +620,27 @@
testPassed(_a + " is >= " + _b);
}
+function shouldBeLessThanOrEqual(_a, _b) {
+ if (typeof _a != "string" || typeof _b != "string")
+ debug("WARN: shouldBeLessThanOrEqual expects string arguments");
+
+ var _exception;
+ var _av;
+ try {
+ _av = eval(_a);
+ } catch (e) {
+ _exception = e;
+ }
+ var _bv = eval(_b);
+
+ if (_exception)
+ testFailed(_a + " should be <= " + _b + ". Threw exception " + _exception);
+ else if (typeof _av == "undefined" || _av > _bv)
+ testFailed(_a + " should be <= " + _b + ". Was " + _av + " (of type " + typeof _av + ").");
+ else
+ testPassed(_a + " is <= " + _b);
+}
+
function expectTrue(v, msg) {
if (v) {
testPassed(msg);
Modified: trunk/Source/WebCore/ChangeLog (264417 => 264418)
--- trunk/Source/WebCore/ChangeLog 2020-07-15 20:18:13 UTC (rev 264417)
+++ trunk/Source/WebCore/ChangeLog 2020-07-15 20:28:06 UTC (rev 264418)
@@ -1,3 +1,23 @@
+2020-07-15 Daniel Bates <daba...@apple.com>
+
+ [iOS] Caret should be before text placeholder instead of after it
+ https://bugs.webkit.org/show_bug.cgi?id=214319
+ <rdar://problem/65295523>
+
+ Reviewed by Wenson Hsieh.
+
+ Move the caret to before the text placeholder to match UIKit.
+
+ Tests: editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-end-of-word.html
+ editing/text-placeholder/caret-before-zero-width-placeholder-in-content-editable-start-of-word.html
+ editing/text-placeholder/insert-into-content-editable-and-remove-via-mutation-event.html
+
+ * editing/Editor.cpp:
+ (WebCore::Editor::insertTextPlaceholder): Compute the position before the placeholder in its parent.
+ This computation can only be done if the placeholder still has a parent. It may not after insertion
+ because arbitrary _javascript_ code can run. If this happens then bail out and return nullptr. Otherwise,
+ set the new selection.
+
2020-07-15 Simon Fraser <simon.fra...@apple.com>
itsnicethat.com page is sometimes non-scrollable
Modified: trunk/Source/WebCore/editing/Editor.cpp (264417 => 264418)
--- trunk/Source/WebCore/editing/Editor.cpp 2020-07-15 20:18:13 UTC (rev 264417)
+++ trunk/Source/WebCore/editing/Editor.cpp 2020-07-15 20:28:06 UTC (rev 264418)
@@ -3323,9 +3323,12 @@
auto placeholder = TextPlaceholderElement::create(document, size);
createLiveRange(*range)->insertNode(placeholder.copyRef());
- VisibleSelection newSelection { positionBeforeNode(placeholder.ptr()), positionAfterNode(placeholder.ptr()) };
- m_document.selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(UserTriggered));
+ // Inserting the placeholder can run arbitrary _javascript_. Check that it still has a parent.
+ if (!placeholder->parentNode())
+ return nullptr;
+ m_document.selection().setSelection(VisibleSelection { positionInParentBeforeNode(placeholder.ptr()), SEL_DEFAULT_AFFINITY }, FrameSelection::defaultSetSelectionOptions(UserTriggered));
+
return placeholder;
}
