Bug 238326

Summary: Add Captive Portal alert to WKWebView
Product: WebKit Reporter: clopez1
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, clopez1, commit-queue, ggaren, kkinnunen, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 238405    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing none

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].