Bug 197950 - [iOS] Select all with existing range selection replaces range instead of selecting all text
Summary: [iOS] Select all with existing range selection replaces range instead of sele...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks: 190571
  Show dependency treegraph
 
Reported: 2019-05-16 10:13 PDT by Daniel Bates
Modified: 2019-06-27 16:06 PDT (History)
3 users (show)

See Also:


Attachments
Patch and layout test (5.99 KB, patch)
2019-05-16 10:24 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To land (6.50 KB, patch)
2019-06-27 16:05 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2019-05-16 10:13:23 PDT
Following the UIKit fix in <rdar://problem/47333786>, UIKit now asks the WebKit whether it can handle Command + A as "select all" BEFORE it tells WebKit to do it. If WebKit saids it cannot perform the "select all" then UIKit does not tell WebKit to do it and WebKit now thinks it should insert the "a".

Steps to reproduce:

1. Visit <data:text/html,<input type="text" value="Select the last word">>.
2. Select the last word in the text field.
3. Press Command + a.

Then the text field's content is updated to read Select the last a. But no modification should have occurred and the entire contents of the field should have been selected.
Comment 1 Daniel Bates 2019-05-16 10:13:31 PDT
<rdar://problem/50245131>
Comment 2 Daniel Bates 2019-05-16 10:24:23 PDT
Created attachment 370048 [details]
Patch and layout test
Comment 3 Wenson Hsieh 2019-05-17 11:06:17 PDT
Comment on attachment 370048 [details]
Patch and layout test

View in context: https://bugs.webkit.org/attachment.cgi?id=370048&action=review

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3000
> +        return !editorState.selectionIsNone && self.hasContent;

This might also mean that the callout bar when there's a ranged selection will now contain "Select All", but I don't think that is necessarily a bad thing :)
Comment 4 Daniel Bates 2019-05-17 16:10:45 PDT
(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 370048 [details]
> Patch and layout test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370048&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3000
> > +        return !editorState.selectionIsNone && self.hasContent;
> 
> This might also mean that the callout bar when there's a ranged selection
> will now contain "Select All", but I don't think that is necessarily a bad
> thing :)

It's a nice feature, but it doesn't match platform conventions. So, I'll need to fix up the patch before landing to keep differentiate between when the keyboard is asking and when the callout bar is asking to give different answers.
Comment 5 Daniel Bates 2019-05-17 16:27:27 PDT
Looks like I'll need to make a UIKit change to be able to know who called canPerformAction
Comment 6 Daniel Bates 2019-06-27 16:04:16 PDT
(In reply to Daniel Bates from comment #5)
> Looks like I'll need to make a UIKit change to be able to know who called
> canPerformAction

No UIKit change necessary callout bar calls -canPerformAction with sender UIMenuController.
Comment 7 Daniel Bates 2019-06-27 16:05:08 PDT
Created attachment 373063 [details]
To land

Do what we do now if -canPerformAction is called by the callout menu code
Comment 8 Daniel Bates 2019-06-27 16:06:55 PDT
Comment on attachment 373063 [details]
To land

Clearing flags on attachment: 373063

Committed r246908: <https://trac.webkit.org/changeset/246908>
Comment 9 Daniel Bates 2019-06-27 16:06:57 PDT
All reviewed patches have been landed.  Closing bug.