Attachment 370048 Details for Bug 197950 – Patch and layout test

[patch] Patch and layout test

bug-197950-20190516102422.patch (text/plain), 5.99 KB, created by Daniel Bates on 2019-05-16 10:24:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2019-05-16 10:24:23 PDT

Size: 5.99 KB

patch

obsolete

>Subversion Revision: 245326
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index b25a8b549bd0b2dd3fc1a5ba0914c60becb9192c..311f931669bc139bc362da7ca585ae05a7285f98 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-05-16  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Select all with existing range selection replaces range instead of selecting all text
>+        https://bugs.webkit.org/show_bug.cgi?id=197950
>+        <rdar://problem/50245131>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Following <rdar://problem/47333786>, UIKit now asks WebKit whether it can handle Command + A as
>+        "select all" instead of just demanding that we handle it. So, WebKit needs to be able to correctly
>+        tell UIKit in advance whether it can handle it. Currenlty WebKit tells UIKit it cannot handle a
>+        "select all" whenever there is an existing range selection. So, UIKit does not tell WebKit to
>+        perform the "select all". Moreover, since UIKit has no other means to handle this key command
>+        itself it tells WebKit the key command was not handled. So, WebKit tells the keyboard to insert
>+        the "a". Instead, WebKit should tell UIKit it can handle a "select all" even when this is an
>+        existing range selection. This fixes the bug and makes WebKit's behavior match platform convention.
>+
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView canPerformActionForWebView:withSender:]):
>+
> 2019-05-15  Alex Christensen  <achristensen@webkit.org>
> 
>         Allow NSFileCoordinator to be called from WebContent process
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index b8aa435a4992228d18cd5812c54c836f3937a6a6..28cfa1898d4bd74df502f361b784f6fc9f295405 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -2997,9 +2997,7 @@ - (BOOL)canPerformActionForWebView:(SEL)action withSender:(id)sender
>     }
> 
>     if (action == @selector(selectAll:)) {
>-        if (!editorState.selectionIsNone && !editorState.selectionIsRange)
>-            return YES;
>-        return NO;
>+        return !editorState.selectionIsNone && self.hasContent;
>     }
> 
>     if (action == @selector(replace:))
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5e14718b91c0279031cb9a65c621886d90a8394d..f23d74d8bee6daf1475ae394d729aebc0697a3d1 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-16  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Select all with existing range selection replaces range instead of selecting all text
>+        https://bugs.webkit.org/show_bug.cgi?id=197950
>+        <rdar://problem/50245131>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test to ensure that pressing Command + A performs a "select all" even when there
>+        is an existing range selection.
>+
>+        * fast/events/ios/select-all-with-existing-selection-expected.txt: Added.
>+        * fast/events/ios/select-all-with-existing-selection.html: Added.
>+
> 2018-12-05  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Add test to ensure that a web page can prevent the default for Command + A
>diff --git a/LayoutTests/fast/events/ios/select-all-with-existing-selection-expected.txt b/LayoutTests/fast/events/ios/select-all-with-existing-selection-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d5573f148cff1ed4f05f0176a56806e3536e0c41
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/select-all-with-existing-selection-expected.txt
>@@ -0,0 +1,11 @@
>+This tests that pressing Command + A selects all the text even when there is an existing range selction. To run this test by hand, press Command + A.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS document.getElementById("test").selectionStart is 0
>+PASS document.getElementById("test").selectionEnd is document.getElementById("test").value.length
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/ios/select-all-with-existing-selection.html b/LayoutTests/fast/events/ios/select-all-with-existing-selection.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2262e8e67cd54e4260037509bd3b46b478664258
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/select-all-with-existing-selection.html
>@@ -0,0 +1,49 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+</head>
>+<body>
>+<p id="description"></p>
>+<div id="console"></div>
>+<input type="text" id="test" value="Select the last word">
>+<script>
>+window.jsTestIsAsync = true;
>+
>+function testSelectAll()
>+{
>+    function checkResultAndDone() {
>+        shouldBeZero('document.getElementById("test").selectionStart');
>+        shouldBe('document.getElementById("test").selectionEnd', 'document.getElementById("test").value.length');
>+
>+        document.body.removeChild(document.getElementById("test"));
>+
>+        finishJSTest();
>+    }
>+    document.addEventListener("selectionchange", checkResultAndDone, { once: true });
>+    if (window.testRunner)
>+        UIHelper.keyDown("a", ["metaKey"]);
>+}
>+
>+function runTest()
>+{
>+    let testElement = document.getElementById("test");
>+    console.assert(testElement.value.indexOf("word") !== -1);
>+    function handleFocus() {
>+        document.addEventListener("selectionchange", testSelectAll, { once: true });
>+        testElement.setSelectionRange(testElement.value.indexOf("word"), testElement.value.length);
>+    }
>+    testElement.addEventListener("focus", handleFocus, { once: true });
>+
>+    if (window.testRunner)
>+        UIHelper.activateElement(testElement);
>+    else
>+        testElement.focus();
>+}
>+
>+description("This tests that pressing Command + A selects all the text even when there is an existing range selction. To run this test by hand, press Command + A.");
>+runTest();
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 197950: 370048 | 373063