[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 #26 from Brad Wright <bwright2 at apple.com> ---
(In reply to comment #25)
> Comment on attachment 303130 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=303130&action=review
> 
> > Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:391
> > +    // dismiss any viewController that is being presented. This is works all VC, popovers, etc.

This is works for all VC.  Missed the for, in the comment.
> 
> Normally we would want to use a capital D in this kind of sentence-style
> comment.
> 
> VC is not normally an abbreviation we use for view controller in WebKit code.
> 
> "This is works all", is not grammatically correct.
> 
> > Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:394
> > +    UIViewController *presentedViewController = [UIViewController _viewControllerForFullScreenPresentationFromView:_view];
> > +    if (presentedViewController)
> > +        [presentedViewController dismissViewControllerAnimated:NO completion:nil];
> 
> Would be nice to define the variable inside the "if" since this is C++ code.
> In fact, I personally would use auto instead of writing out UIViewController
> * for the type of the variable.
> 
> > Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:508
> > +    // Clear out any previous view controller reference, to prevent calling dismiss on a view controller that
> > +    // was no longer being presented. Don't save a reference to the UIDocumentMenuViewController as it is self dismissing.
> > +    _presentationViewController = nil;
> 
> I don’t understand this comment and therefore don’t understand the code.

So what happening is that the _presentationViewController is used to called in:

- (void)_dismissDisplayAnimated:(BOOL)animated
{
    if (_presentationPopover) {
        [_presentationPopover dismissPopoverAnimated:animated];
        [_presentationPopover setDelegate:nil];
        _presentationPopover = nil;
    }

    if (_presentationViewController) {
        [_presentationViewController dismissViewControllerAnimated:animated completion:^{
            _presentationViewController = nil;
        }];
    }
}


The UIDocumentMenuViewController dismisses itself, so when _presentationViewController dismissViewControllerAnimated:animated is called in this function, it dismisses the WKWebView when is presented modally.  That's why it's set to nil in this function, so that the errant code path is never executed.
> 
> On iPhone, the method called just before this will call
> _presentFullscreenViewController. Then this line of code will clear out the
> reference to that just-presented view controller. Given that, why does this
> comment talk about clearing out a "previous view controller reference"?

-- 
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/20170301/476e985f/attachment.html>