Cleanup canPerformActionForWebView in relation to the webSelectionAssistant being removed
Created attachment 340147 [details] Patch
<rdar://problem/40147338>
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?
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.
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.
Created attachment 340220 [details] Patch
https://trac.webkit.org/changeset/231726/webkit