Bug 162657 – Make it possible to test web-related user-interface features

• Component: New Bugs
• Status: RESOLVED FIXED

[Megan Gardner]

Test for long press on image

[Megan Gardner]

Created attachment 290050 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 290050 [details]
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.

[Megan Gardner]

Created attachment 290258 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 290258 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        Test for long press on image

This title isn't really accurate. It's "Make it possible to test web-related user-interface features" or something.

> Source/WebKit2/ChangeLog:11
> +        Wrote a test for long press on an image, and used the sheet scraping functionality

s/Wrote/Add

> Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.h:75
> +- (NSArray *)currentAvailableActionStrings;

"strings" is a bit vague. "titles"?

> Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:224
> +    for (UIAlertAction *action in _interactionSheet.get().actions)
> +        [array addObject:action.title];

This gets localized text. Is this OK, or should we stringify the action enum values?

> LayoutTests/fast/events/touch/ios/long-press-on-image.html:33
> +					var actionDictionary = JSON.parse(result);

Please de-tab the file.

> LayoutTests/fast/events/touch/ios/long-press-on-image.html:42
> +						output += 'Available Actions: <br>';
> +						var arrayLength = actionStrings.length;
> +						for (var i = 0; i < arrayLength; i++) {
> +						    output += actionStrings[i] + '<br>';
> +						}

You could do this with an Array.join(separator).

[Megan Gardner]

Created attachment 290353 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 290353 [details]
Patch

Clearing flags on attachment: 290353

Committed r206645: <http://trac.webkit.org/changeset/206645>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.