[Webkit-unassigned] [Bug 165225] UIViewController with WkWebView presented modally causes the presented UIViewController to be dismissed.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=165225

--- Comment #6 from Brad Wright <bwright2 at apple.com> ---
(In reply to comment #5)
> Comment on attachment 295877 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295877&action=review
> 
> > Source/WebKit2/ChangeLog:3
> > +        UIViewController with WkWebView presented modally causes the presented UIViewController to be dismissed.
> 
> This title seems insufficient. It seems the issues is specific to File
> Upload so that should be mentioned.
> 
> Perhaps a better title would be:
> 
>     WKWebView file upload dialog may incorrectly dismiss unrelated modal
> view controller
> 
> Does that sound accurate to you?
> 
> > Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:557
> > -    if (UICurrentUserInterfaceIdiomIsPad())
> > +    if (UICurrentUserInterfaceIdiomIsPad()) {
> >          [self _presentPopoverWithContentViewController:viewController animated:YES];
> > -    else
> > -        [self _presentFullscreenViewController:viewController animated:YES];
> > +    } else {
> 
> Style: WebKit's Style guideline is to not use braces for single statement
> conditional blocks. So these braces for the if should not be added. The
> else, which is multiline should have braces.
> https://webkit.org/code-style-guidelines/#braces-one-line
> 
> > Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:560
> > +        _presentationViewController = nil;
> 
> Hmm, this doesn't seem right.
> 
>   - The previous code attempts to dismiss the old view controller it
> presented and is then presenting the new view controller.
>   - The new code ignores the old view controller and presents the new view
> controller.

You don't need to dismiss this view controller because it dismisses itself. By explicitly calling another dismiss, the code is a duplicate dismiss which then removes the modal view controller.

This problem will never appear in Safari, because there are no modally presented views.  If you run the same sample project that I uploaded, you will see that the modal view controller is not dismissed.  This is because the popup VC code path does not call dismissViewControllerAnimated.  The problem only happens on a modal VC with a WkWebView and on the iPhone.

> 
> Shouldn't we continue to dismiss the old view controller? if
> _presentationViewController was the incorrect view controller to be
> dismissing (an ancestor), then perhaps _presentationViewController's
> presentedViewController would be the correct view controller to dismiss
> instead of dismissing nothing.
> 
> What do you think?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161202/7b6db90d/attachment-0001.html>