Bug 234017 - [iOS] Add SPI to enable find interactions on WKWebView
Summary: [iOS] Add SPI to enable find interactions on WKWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-08 10:25 PST by Aditya Keerthi
Modified: 2021-12-10 11:57 PST (History)
6 users (show)

See Also:


Attachments
Patch (11.87 KB, patch)
2021-12-08 11:19 PST, Aditya Keerthi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aditya Keerthi 2021-12-08 10:25:05 PST
...
Comment 1 Aditya Keerthi 2021-12-08 10:25:42 PST
rdar://86140542
Comment 2 Aditya Keerthi 2021-12-08 11:19:06 PST
Created attachment 446392 [details]
Patch
Comment 3 Wenson Hsieh 2021-12-09 15:25:37 PST
Comment on attachment 446392 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:445
> +@property (nonatomic, readwrite, setter=_setFindInteractionEnabled:) BOOL _findInteractionEnabled WK_API_AVAILABLE(ios(WK_IOS_TBA));

There was some discussion about whether this should be here (on WKWebView) or WKPreferences.

We (Geoff, Tim, Aditya, Alex and I) chatted about this over Slack, and I *think* we all eventually came to the conclusion that this property should go on WKWebView itself, since setting this has side effects beyond just changing the value of a flag (and so calling `-_setFindInteractionEnabled:` is, in a way, performing an action on the web view).

If it were only changing the flag's value, then I think we would put this on WKPreferences, like most of the other API/SPI flags.
Comment 4 EWS 2021-12-09 18:17:43 PST
Committed r286823 (245057@main): <https://commits.webkit.org/245057@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446392 [details].
Comment 5 Darin Adler 2021-12-10 11:57:24 PST
Comment on attachment 446392 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:457
> +    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)]);

Not critical to fix, but this could be:

    auto webView = ...

or:

    RetainPtr webView = ...

Either of those seems slightly nicer.