Bug 238326 - Add Captive Portal alert to WKWebView
Summary: Add Captive Portal alert to WKWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on: 238405
Blocks:
  Show dependency tree / graph
 
Reported: 2022-03-24 07:57 PDT by clopez1
Modified: 2022-03-29 13:41 PDT (History)
8 users (show)

See Also:

Attachments
Patch (2.37 KB, patch)
2022-03-24 08:51 PDT, clopez1 		no flags Details | Formatted Diff | Diff
Patch for landing (3.31 KB, patch)
2022-03-29 12:03 PDT, Brent Fulgham 		no flags Details | Formatted Diff | Diff
Patch for landing (2.49 KB, patch)
2022-03-29 12:14 PDT, Brent Fulgham 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description clopez1 2022-03-24 07:57:05 PDT 
Add a Captive Portal alert in WKWebView
Comment 1 clopez1 2022-03-24 08:51:42 PDT 
Created attachment 455643 [details]
Patch
Comment 2 Geoffrey Garen 2022-03-24 09:46:21 PDT 
Comment on attachment 455643 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:1560
> +    [self _presentCaptivePortalModeAlertIfNeeded];

Views can move in and out of windows frequently, for example I believe they do so when switching tabs. I recommend double-checking that _presentCaptivePortalModeAlertIfNeeded is super efficient in the "do nothing" case.
Comment 3 clopez1 2022-03-25 10:12:43 PDT 
Comment on attachment 455643 [details]
Patch

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

>> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:1560
>> +    [self _presentCaptivePortalModeAlertIfNeeded];
> 
> Views can move in and out of windows frequently, for example I believe they do so when switching tabs. I recommend double-checking that _presentCaptivePortalModeAlertIfNeeded is super efficient in the "do nothing" case.

Agreed. `_presentCaptivePortalModeAlertIfNeeded` should be efficient, especially for subsequent calls.
Comment 4 Brent Fulgham 2022-03-25 12:08:18 PDT 
Comment on attachment 455643 [details]
Patch

The iOS tests are failing for other patches -- I don't see how they are related to this change. Re-adding commit-queue.
Comment 5 EWS 2022-03-25 13:48:17 PDT 
Committed r291884 (248883@main): <https://commits.webkit.org/248883@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455643 [details].
Comment 6 Radar WebKit Bug Importer 2022-03-25 13:49:18 PDT 
<rdar://problem/90854781>
Comment 7 Simon Fraser (smfr) 2022-03-25 20:01:46 PDT 
This broke the internal iOS build.
Comment 8 WebKit Commit Bot 2022-03-25 20:02:30 PDT 
Re-opened since this is blocked by bug 238405
Comment 9 Brent Fulgham 2022-03-29 11:16:42 PDT 
Comment on attachment 455643 [details]
Patch

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

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.h:28
> +#if USE(APPLE_INTERNAL_SDK) && PLATFORM(IOS_FAMILY)

I think this might avoid breaking the build:

#if USE(APPLE_INTERNAL_SDK) && PLATFORM(IOS_FAMILY) && __has_include(<WebKitAdditions/WKWebViewAdditions.h>)
Comment 10 Brent Fulgham 2022-03-29 12:03:04 PDT 
Created attachment 456048 [details]
Patch for landing
Comment 11 EWS 2022-03-29 12:13:20 PDT 
ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!.
Comment 12 Brent Fulgham 2022-03-29 12:14:40 PDT 
Created attachment 456049 [details]
Patch for landing
Comment 13 EWS 2022-03-29 13:40:58 PDT 
Committed r292065 (248999@main): <https://commits.webkit.org/248999@main>

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