[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 #5 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 295877
  --> https://bugs.webkit.org/attachment.cgi?id=295877
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?

-- 
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/20161201/2052f079/attachment.html>