[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 #8 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.
> 
> 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?

If you run the included example project on the iPhone, you will see the WkWebView get's dismissed when it shouldn't.  Then try running on the iPad, and you will see the WkWebView doesn't get dismissed.  This is because there is no extra call to dismissViewControllerAnimated.

Every time you call, dissmissViewControllerAnimaged it just removes another ViewController from the stack of modally presented View Controllers.  UIDocumentMenuViewController already removes itself from the stack automatically.  Thus this duplicate call, is dismissing the wrong ViewController.

-- 
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/dcccf473/attachment-0001.html>