[webkit-reviews] review granted: [Bug 196265] Add basic layout test coverage for File Picker on iOS : [Attachment 365989] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 196265: Add basic layout test coverage for File Picker on iOS
https://bugs.webkit.org/show_bug.cgi?id=196265

Attachment 365989: Patch

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




--- Comment #3 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 365989
  --> https://bugs.webkit.org/attachment.cgi?id=365989
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:7018
> +	   auto* result = @{ userInterfaceItem: [_fileUploadPanel
currentAvailableActionTitles] };

Nit - would be cleaner just to return @{ ~ }

> Tools/WebKitTestRunner/ios/TestControllerIOS.mm:53
> +static void overridePresentViewController()

Nit - maybe overridePresentViewControllerOrPopover, since it's used for both
purposes?

> LayoutTests/fast/forms/ios/file-upload-panel.html:62
> +	   UIHelper.activateElement(testInput).then(() => {
> +	       getFileUploadPickerMenuItems().then((_items) => {
> +		   items = _items;
> +		   shouldBeTrue("areArraysEqual(items, " + expectedMenuItems +
")");
> +		   debug("");
> +		   dismissFilePicker().then(() => { resolve(); });
> +	       });
> +	   });

Nit - IMO this function could be made cleaner using async - await, but either
way it's fine.