Bug 185536 – Cleanup canPerformActionForWebView in relation to the webSelectionAssistant being removed

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Cleanup canPerformActionForWebView in relation to the webSelectionAssistant being removed

[Megan Gardner]

Created attachment 340147 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/40147338>

[Tim Horton]

Comment on attachment 340147 [details]
Patch

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

> Source/WebKit/ChangeLog:8
> +        The _webSelectionAssistant is now always nil, therefor many of these checks are unnecessary.

therefore

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-2322
> -        // Don't attempt selectAll with general web content.

I’m confused... did this behavior (where we enable selectAll) change with your earlier change to always use the modern selection path?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-2325
> -        // FIXME: Only enable if the selection doesn't already span the entire document.

Why’d this FIXME go away?

[Wenson Hsieh]

Comment on attachment 340147 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:2271
> +        if (_page->editorState().isInPasswordField || _page->editorState().selectionIsRange)

I think you're missing a ! here.

[Megan Gardner]

Comment on attachment 340147 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:2271
>> +        if (_page->editorState().isInPasswordField || _page->editorState().selectionIsRange)
> 
> I think you're missing a ! here.

you're right, thanks!

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-2322
>> -        // Don't attempt selectAll with general web content.
> 
> I’m confused... did this behavior (where we enable selectAll) change with your earlier change to always use the modern selection path?

It didn't really change, as this logic was overly complicated and _page->editorState().selectionIsRange is basically equivalent to hasWebSelection, especially in the new paradigm. This is just overly complicated so I wanted to clean this up separately to removing _webSelectionAssistant. This new logic basically says if you have a selection, and it's not ranged, which basically means if you have a caret, then allow SelectAll. You cannot have a selection that is not ranged in nonEditable text, so we only allow selectAll in editable content where there is no selection.

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-2325
>> -        // FIXME: Only enable if the selection doesn't already span the entire document.
> 
> Why’d this FIXME go away?

Select All actually only appears when you have a caret selection, so you can only go from zero to everything, we don't even consider expanding via select all with a smaller selection. This mirrors Note's behavior.

[Megan Gardner]

Created attachment 340220 [details]
Patch

[Megan Gardner]

https://trac.webkit.org/changeset/231726/webkit