Changeset 246908 in webkit

Timestamp:

Jun 27, 2019 4:06:52 PM (5 years ago)

Author:

dbates@webkit.org

Message:

[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 Wenson Hsieh.

Source/WebKit:

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. However we need to keep the current logic just for when UIKit is
asking us with respect to populating the callout menu to not regress platform behavior.

• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView canPerformActionForWebView:withSender:]): Do what we do now if we are called
when populating the callout menu and action is Select All. Otherwise, return YES for the Select All
action if we have a non-empty selection.

LayoutTests:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/select-all-with-existing-selection-expected.txt (added)
• LayoutTests/fast/events/ios/select-all-with-existing-selection.html (added)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-06-27  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 Wenson Hsieh.
+
+        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.
+
 2019-06-27  Russell Epstein  <russell_e@apple.com>
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-27  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 Wenson Hsieh.
+
+        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. However we need to keep the current logic just for when UIKit is
+        asking us with respect to populating the callout menu to not regress platform behavior.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView canPerformActionForWebView:withSender:]): Do what we do now if we are called
+        when populating the callout menu and action is Select All. Otherwise, return YES for the Select All
+        action if we have a non-empty selection.
+
 2019-06-27  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -3052 +3052 @@
 
     if (action == @selector(selectAll:)) {
-        if (!editorState.selectionIsNone && !editorState.selectionIsRange)
-            return YES;
-        return NO;
+        // By platform convention we don't show Select All in the callout menu for a range selection.
+        if ([sender isKindOfClass:UIMenuController.class])
+            return !editorState.selectionIsNone && !editorState.selectionIsRange;
+        return !editorState.selectionIsNone && self.hasContent;
     }