Bug 181884 – Present WKFileUploadPanel on correct ViewController

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Present WKFileUploadPanel on correct ViewController

[Megan Gardner]

Created attachment 331793 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 331793 [details]
Patch

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

Looks good to me.

There are other places that use -[UIViewController _viewControllerForFullScreenPresentationFromView:]. For example:
UIProcess/ios/forms/WKAirPlayRoutePicker.mm

Should they change as well?

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462
> +    NSLog(@"Failed to find a view controller to show form validation popover");

Accidental log.

[Tim Horton]

(In reply to Joseph Pecoraro from comment #2)
> Comment on attachment 331793 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=331793&action=review
> 
> Looks good to me.
> 
> There are other places that use -[UIViewController
> _viewControllerForFullScreenPresentationFromView:]. For example:
> UIProcess/ios/forms/WKAirPlayRoutePicker.mm
> 
> Should they change as well?

Seems likely.
 
> > Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462
> > +    NSLog(@"Failed to find a view controller to show form validation popover");
> 
> Accidental log.

Humorously that’s in the other version of this function.

[Tim Horton]

Comment on attachment 331793 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:456
> +static UIViewController *fallbackViewController(UIView *view)

I wonder if we should put this in the UIDelegate implementation if the client returns null, so we don’t have to put it everywhere.

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:471
> +        _presentationViewController = [self.delegate viewControllerForPresentingFileUploadPanel:self];

What if this returns nil? Can it?

[Tim Horton]

Comment on attachment 331793 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:456
>> +static UIViewController *fallbackViewController(UIView *view)
> 
> I wonder if we should put this in the UIDelegate implementation if the client returns null, so we don’t have to put it everywhere.

In a followup!

>>> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:462
>>> +    NSLog(@"Failed to find a view controller to show form validation popover");
>> 
>> Accidental log.
> 
> Humorously that’s in the other version of this function.

Maybe RELEASE_LOG instead

>> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:471
>> +        _presentationViewController = [self.delegate viewControllerForPresentingFileUploadPanel:self];
> 
> What if this returns nil? Can it?

(I think you want if !presentationViewController instead of an else)

[Megan Gardner]

https://trac.webkit.org/changeset/227247/webkit

[Radar WebKit Bug Importer]

<rdar://problem/36673774>

[Megan Gardner]

InRadar <rdar://problem/35114892>

[Megan Gardner]

<rdar://problem/35114892>