Bug 225260 - App highlight UI should be disabled when selecting text in image overlays
Summary: App highlight UI should be disabled when selecting text in image overlays
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-30 16:59 PDT by Wenson Hsieh
Modified: 2021-04-30 20:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (22.65 KB, patch)
2021-04-30 17:35 PDT, Wenson Hsieh
thorton: review+
Details | Formatted Diff | Diff
For EWS (22.79 KB, patch)
2021-04-30 18:52 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-04-30 16:59:52 PDT
rdar://77359313
Comment 1 Wenson Hsieh 2021-04-30 17:35:47 PDT
Created attachment 427471 [details]
Patch
Comment 2 Tim Horton 2021-04-30 18:43:19 PDT
Comment on attachment 427471 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3571
> +        // FIXME: It doesn't seem like this codepath is exercised, since UIKit only asks for the action target for custom actions
> +        // added via -[UIMenuController setMenuItems:]. Can we remove this check?

Yikes, good catch.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3769
> +        return state.selectionIsRange && !state.isContentEditable && !state.selectionIsRangeInsideImageOverlay ? self : nil;

Please factor this out as discussed.

> Tools/ChangeLog:21
> +        Add support for a new `TestOption` that enables app higlights. See the new layout test for more information.

sp. "highlights"
Comment 3 Wenson Hsieh 2021-04-30 18:46:31 PDT
Comment on attachment 427471 [details]
Patch

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

Thanks for the review!

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3769
>> +        return state.selectionIsRange && !state.isContentEditable && !state.selectionIsRangeInsideImageOverlay ? self : nil;
> 
> Please factor this out as discussed.

👍🏻

>> Tools/ChangeLog:21
>> +        Add support for a new `TestOption` that enables app higlights. See the new layout test for more information.
> 
> sp. "highlights"

Whoops, fixed!
Comment 4 Wenson Hsieh 2021-04-30 18:52:12 PDT
Created attachment 427482 [details]
For EWS
Comment 5 EWS 2021-04-30 20:27:02 PDT
Committed r276871 (237217@main): <https://commits.webkit.org/237217@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427482 [details].