Bug 185257

Summary: A WKWebView in a UIViewController that is presented modally and uses a file picker will be incorrectly dismissed by the system
Product: WebKit Reporter: Zach Waugh <zwaugh>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, obarnett, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari 11   
Hardware: iPhone / iPad   
OS: iOS 11   
Attachments:
Description Flags
Sample project that reproduces the bug
none
Patch
none
Patch
none
Patch
none
Patch none

Description Zach Waugh 2018-05-03 10:00:19 PDT
Created attachment 339421 [details]
Sample project that reproduces the bug

This is similar to https://bugs.webkit.org/show_bug.cgi?id=165225. When a WKWebView is presented in a modal UIViewControlller and a file picker is displayed (from <input type="file">), taking any actions from within the file browser will result in the file browser modal being dismissed as well as the application modal. The expected result is only the file browser modal is dismissed and the application modal remains.

To Reproduce:
1. Present a UIViewController containing a WKWebView modally
2. The WKWebView should have an <input type="file"> element.
3. Click the "Choose File" button
4. Choose "Browse" from the action sheet that is displayed
5. iOS will present the file browser modally
6. When the file browser modal is displayed, either choose "Cancel" in the top-right or select a file
7. Upon either of those actions, the file browser modal will be dismissed as well as this application modal that contains the web view
8. The expected behavior is only the file browser modal is dismissed and the application modal remains

I've attached a sample project that reproduces this in iOS 11.3, which is also available on GitHub here with an example GIF: https://github.com/zachwaugh/wkwebview-bugs/tree/master/ModalFileDismissal
Comment 1 Radar WebKit Bug Importer 2018-06-05 12:47:42 PDT
<rdar://problem/40819252>
Comment 2 Tim Horton 2018-06-05 15:41:06 PDT

*** This bug has been marked as a duplicate of bug 183549 ***
Comment 3 Zach Waugh 2018-06-06 07:35:36 PDT
I can't see the full details of the linked bug, but I don't believe they are the same issue. This bug report is concerning when the UIDocumentBrowserViewController is presented and subsequently dismissed. If you click through to this link (https://github.com/zachwaugh/wkwebview-bugs/tree/master/ModalFileDismissal), there is gif showing the full bug and it's unrelated to the action sheet.

I've confirmed the bug is still present in iOS 12 beta 1 building against Xcode 10 beta 1.
Comment 4 Tim Horton 2018-06-06 07:53:54 PDT
Interesting! I bet it has a similar fix, then.
Comment 5 Tim Horton 2018-06-06 11:10:07 PDT
It does look like WKFileUploadPanel has the same bug.
Comment 6 Olivia Barnett 2018-07-03 13:51:23 PDT
Created attachment 344211 [details]
Patch
Comment 7 Olivia Barnett 2018-07-03 13:53:45 PDT
Created attachment 344212 [details]
Patch
Comment 8 Tim Horton 2018-07-03 13:57:44 PDT
Comment on attachment 344212 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:288
> +            [currentPresentedViewController dismissViewControllerAnimated:YES completion:nil];

I think you probably still want to clear _presentationViewController in the completion handler.
Comment 9 Tim Horton 2018-07-03 13:59:14 PDT
Comment on attachment 344212 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:288
>> +            [currentPresentedViewController dismissViewControllerAnimated:YES completion:nil];
> 
> I think you probably still want to clear _presentationViewController in the completion handler.

(Otherwise I think this is a leak?)
Comment 10 Olivia Barnett 2018-07-03 14:42:35 PDT
Created attachment 344222 [details]
Patch
Comment 11 Tim Horton 2018-07-03 14:50:05 PDT
Comment on attachment 344222 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:288
> +            [currentPresentedViewController dismissViewControllerAnimated:YES completion:^{_presentationViewController = nil;

The first statement inside the block should be on the next line.
Comment 12 Olivia Barnett 2018-07-03 14:58:37 PDT
Created attachment 344225 [details]
Patch
Comment 13 WebKit Commit Bot 2018-07-04 02:43:15 PDT
Comment on attachment 344225 [details]
Patch

Clearing flags on attachment: 344225

Committed r233502: <https://trac.webkit.org/changeset/233502>
Comment 14 WebKit Commit Bot 2018-07-04 02:43:16 PDT
All reviewed patches have been landed.  Closing bug.