[webkit-reviews] review granted: [Bug 208687] Adopt UIContextMenu for WKFileUploadPanel : [Attachment 392896] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Mar 7 14:13:51 PST 2020


Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 208687: Adopt UIContextMenu for WKFileUploadPanel
https://bugs.webkit.org/show_bug.cgi?id=208687

Attachment 392896: Patch

https://bugs.webkit.org/attachment.cgi?id=392896&action=review




--- Comment #11 from Tim Horton <thorton at apple.com> ---
Comment on attachment 392896
  --> https://bugs.webkit.org/attachment.cgi?id=392896
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:427
> +	   if (!strongSelf)
> +	       return [UIMenu menuWithTitle:@"" children:@[]];

it is valid to return a nil menu, right? Wouldn't that be more appropriate?

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:477
> +- (void)_showFilePickerMenu

The underscores!

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:488
> +- (void)_showDocumentPickerMenu

Again!

> Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:493
> +    BOOL containsImageMediaType = !mediaTypes.count ||
arrayContainsUTIThatConformsTo(mediaTypes, kUTTypeImage);
> +    BOOL containsVideoMediaType = !mediaTypes.count ||
arrayContainsUTIThatConformsTo(mediaTypes, kUTTypeMovie);

Is this "contains", or maybe "allows"?

These are unused on Catalyst, maybe move them inside the #else?


More information about the webkit-reviews mailing list