Bug 181841

Summary: Use promises for basic-gestures.js and 'await' for the corresponding tests
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: New BugsAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, megan_gardner, rniwa, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 181798    
Bug Blocks: 181681, 173833    
Attachments:
Description Flags
Patch
megan_gardner: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews101 for mac-sierra none

Description Frédéric Wang (:fredw) 2018-01-19 00:55:52 PST
Use promises for basic-gestures.js and 'await' for the corresponding tests
Comment 1 Frédéric Wang (:fredw) 2018-01-19 01:01:50 PST
Created attachment 331715 [details]
Patch
Comment 2 EWS Watchlist 2018-01-19 02:02:30 PST
Comment on attachment 331715 [details]
Patch

Attachment 331715 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6132466

New failing tests:
media/modern-media-controls/media-controller/media-controller-fade-controls-when-entering-fullscreen.html
Comment 3 EWS Watchlist 2018-01-19 02:02:31 PST
Created attachment 331717 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 4 Frédéric Wang (:fredw) 2018-01-19 02:07:31 PST
The failure on mac seems unrelated as it happened without the patch: https://webkit-queues.webkit.org/patch/331715/mac-ews
Comment 5 Megan Gardner 2018-01-19 08:30:09 PST
Comment on attachment 331715 [details]
Patch

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

Looks good other than the few formatting things, and the seemingly unneeded 'done' return.

> LayoutTests/fast/events/touch/ios/long-press-then-drag-left-to-change-selected-text.html:69
> +                output += 'FAIL: failed to reduce selection to a single character by dragging right. Incorrect Selection: ' +     document.getSelection().toString();

extraneous spaces after +
Looks like they were there in the original, but should still be removed.

> LayoutTests/fast/events/touch/ios/long-press-then-drag-to-select-text.html:37
> +                output += 'FAIL: failed to reduce selection to a single character by dragging down. Incorrect Selection: ' +     document.getSelection().toString();

extraneous spaces after +

> LayoutTests/fast/events/touch/ios/long-press-then-drag-up-to-change-selected-text.html:69
> +                output += 'FAIL: failed to reduce selection to a single character by dragging down. Incorrect Selection: ' +     document.getSelection().toString();

Extraneous spaces after +

> LayoutTests/resources/basic-gestures.js:7
> +                    uiController.uiScriptComplete('Done');

Is there a reason that this function now returns 'Done'?
Comment 6 Frédéric Wang (:fredw) 2018-01-19 08:35:25 PST
Comment on attachment 331715 [details]
Patch

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

>> LayoutTests/fast/events/touch/ios/long-press-then-drag-left-to-change-selected-text.html:69
>> +                output += 'FAIL: failed to reduce selection to a single character by dragging right. Incorrect Selection: ' +     document.getSelection().toString();
> 
> extraneous spaces after +
> Looks like they were there in the original, but should still be removed.

Right, I'll fix these.

>> LayoutTests/resources/basic-gestures.js:7
>> +                    uiController.uiScriptComplete('Done');
> 
> Is there a reason that this function now returns 'Done'?

mmh, no. I think I initially copied it from ui-helper.js but that should not affect the tests here.
Comment 7 Wenson Hsieh 2018-01-19 09:01:29 PST
Comment on attachment 331715 [details]
Patch

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

r=me

> LayoutTests/resources/basic-gestures.js:232
> +                 uiController.uiScriptComplete();

Nit - not a new issue, but the indentation level is off on all of these:

```
uiController.sendEventStream(JSON.stringify(eventStream), function() {});
    uiController.uiScriptComplete();
})();`, resolve);
```
Comment 8 Frédéric Wang (:fredw) 2018-01-19 09:07:09 PST
Just for future reference:  Megan Gardner was actually not a reviewer when I landed this patch. webkit-patch displayed a warning but I could not abort it in time. So the commit is

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

and "reviewed by Wenson Hsieh".

(In reply to Wenson Hsieh from comment #7)
> Nit - not a new issue, but the indentation level is off on all of these:
> 
> ```
> uiController.sendEventStream(JSON.stringify(eventStream), function() {});
>     uiController.uiScriptComplete();
> })();`, resolve);
> ```

Right, but the patch already landed so I prefer not to send another one for whitespace-only change.
Comment 9 Radar WebKit Bug Importer 2018-01-19 09:09:13 PST
<rdar://problem/36660038>