[Webkit-unassigned] [Bug 162657] Test for long press on image

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 29 14:30:16 PDT 2016


https://bugs.webkit.org/show_bug.cgi?id=162657

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #290050|review?                     |review-
              Flags|                            |

--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 290050
  --> https://bugs.webkit.org/attachment.cgi?id=290050
Patch

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

r- for the build breakage, but looks good. The actual new test is missing, though.

> Source/WebKit2/ChangeLog:10
> +        Wrote a test for long press on an image, and used the sheet scraping functionality
> +        to make sure that the correct actions were being displayed.

You might want to reference the revision that added the code you're now testing.

> Source/WebKit2/ChangeLog:12
> +        Reviewed by NOBODY (OOPS!).

This line should go above the description

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:266
> +- (NSDictionary *)contentsOfUserInterfaceItem:(NSString *)userInterfaceItem WK_API_AVAILABLE(ios(WK_IOS_TBA));

This should have a leading underscore, as we do for all private properties and functions (as should the ones above, which I'll fix later).

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:234
>  - (void)selectFormAccessoryPickerRow:(NSInteger)rowIndex;
> +- (NSDictionary *)contentsOfUserInterfaceItem:(NSString *)userInterfaceItem;

We should use underscores here as well; put one on contentsOfUserInterfaceItem.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3855
> +        return @{userInterfaceItem:[_actionSheetAssistant currentAvailableActionStrings]};

Preferred spacing is:
return @{ userInterfaceItem: [_actionSheetAssistant currentAvailableActionStrings] };

> Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:77
> +    JSObjectRef contentsOfUserInterfaceItem(JSStringRef item) const;

No need for the "item" parameter name.

> Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:65
> +    // Returned object has a list of string the describe actions

Comment isn't quite accurate: its'a dictionary with a key matching interfaceItem which might have arbitrary content inside.

-- 
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/20160929/f83f5485/attachment.html>


More information about the webkit-unassigned mailing list