.
Created attachment 331385 [details] Patch
Comment on attachment 331385 [details] Patch Attachment 331385 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6093535 New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html
Created attachment 331388 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 331385 [details] Patch I think there could be some more interesting integrations with basic-gestures.js as well.
(In reply to Megan Gardner from comment #4) > Comment on attachment 331385 [details] > Patch > > I think there could be some more interesting integrations with > basic-gestures.js as well. OK, I think most of these tests could use touchAndDragFromPointToPoint and tapAtPoint (some of them currently seems to assume that the script completes with "Done", so they might need some adjustment). However, it is a shame that: - We duplicate some code in ui-helper.js and basic-gesture.js (e.g. UIHelper::tapAt and tapAtPoint). - basic-gestures.js does not return promises running the scripts. This makes the usage a bit more verbose and requires more indentation than with "await".
(In reply to Frédéric Wang (:fredw) from comment #5) > (In reply to Megan Gardner from comment #4) > > Comment on attachment 331385 [details] > > Patch > > > > I think there could be some more interesting integrations with > > basic-gestures.js as well. > > OK, I think most of these tests could use touchAndDragFromPointToPoint and > tapAtPoint (some of them currently seems to assume that the script completes > with "Done", so they might need some adjustment). However, it is a shame > that: > > - We duplicate some code in ui-helper.js and basic-gesture.js (e.g. > UIHelper::tapAt and tapAtPoint). > - basic-gestures.js does not return promises running the scripts. This makes > the usage a bit more verbose and requires more indentation than with "await". This is true. What do you think about building UIHelper functionality off of basic-gestures.js? (i.e. use the helpers in basic-gestures.js to generate UI-side scripts that we call runUIScript with in ui-helper.js?)
(In reply to Wenson Hsieh from comment #6) > This is true. What do you think about building UIHelper functionality off of > basic-gestures.js? (i.e. use the helpers in basic-gestures.js to generate > UI-side scripts that we call runUIScript with in ui-helper.js?) Yes, I was thinking about it too and I believe that should work. @Megan: What do you suggest?
Yes, looking at these a bit more, I think that is the thing to do. Use the strings generated by basic-gestures.js to run the encapsulated async functions in ui-helper. There is probably more streamlining and cleanup to be done here, but it would take a longer look.
I was thinking maybe I should upload a preliminary patch to move the files in LayoutTests/fast/events/touch/ios/resources/ into LayoutTests/resources/ so that they are in the same location as ui-helper.js and other helper scripts?
(In reply to Frédéric Wang (:fredw) from comment #9) > I was thinking maybe I should upload a preliminary patch to move the files > in LayoutTests/fast/events/touch/ios/resources/ into LayoutTests/resources/ > so that they are in the same location as ui-helper.js and other helper > scripts? That sounds like a good idea to me!
I opened bug 181798. However, I'm wondering if we should just merge basic-gestures.js into ui-helper.js (ui-debugging.js does not seem to be used). There are not too many tests using this file, so I guess I can convert them to the promise/await syntax.
Note that the intent of ui-helper.js to abstract away the difference between WK1 and WK2 so whatever we do here should work in both WK1 and WK2, and shouldn't create two different solutions for them.
(In reply to Ryosuke Niwa from comment #12) > Note that the intent of ui-helper.js to abstract away the difference between > WK1 and WK2 so whatever we do here should work in both WK1 and WK2, and > shouldn't create two different solutions for them. Also the difference between macOS and iOS. i.e. tap vs click, etc...
(In reply to Ryosuke Niwa from comment #13) > (In reply to Ryosuke Niwa from comment #12) > > Note that the intent of ui-helper.js to abstract away the difference between > > WK1 and WK2 so whatever we do here should work in both WK1 and WK2, and > > shouldn't create two different solutions for them. > > Also the difference between macOS and iOS. i.e. tap vs click, etc... In the long term, I think we also want to abstract out cross-platform differences where possible, which is why we use some generic names like "activateAt", which simulates a mouseDown/mouseUp on macOS but sends a tap on iOS.
(In reply to Ryosuke Niwa from comment #12) > Note that the intent of ui-helper.js to abstract away the difference between > WK1 and WK2 so whatever we do here should work in both WK1 and WK2, and > shouldn't create two different solutions for them. Per the changes made in attachment 331385 [details], uiController.dragFromPointToPoint is already used for tests on iOS and macOS. I am not sure whether it works on WK1 and how to emulate the behavior if it does not. I'm currently implemeting iOS frame scrolling in 173833 and I will only need iOS/WK2. So if it's too complicated I guess I can just use user-gestures.js for now. I uploaded bug 181841 to move it to the more concise syntax with promise/await. Or maybe I can do some asserts as other functions do to ensure they are only used on WK2 for now.
(In reply to Frédéric Wang (:fredw) from comment #15) > (In reply to Ryosuke Niwa from comment #12) > > Note that the intent of ui-helper.js to abstract away the difference between > > WK1 and WK2 so whatever we do here should work in both WK1 and WK2, and > > shouldn't create two different solutions for them. > > Per the changes made in attachment 331385 [details], > uiController.dragFromPointToPoint is already used for tests on iOS and > macOS. I am not sure whether it works on WK1 and how to emulate the behavior > if it does not. I'm currently implemeting iOS frame scrolling in 173833 and > I will only need iOS/WK2. So if it's too complicated I guess I can just use > user-gestures.js for now. I uploaded bug 181841 to move it to the more > concise syntax with promise/await. Or maybe I can do some asserts as other > functions do to ensure they are only used on WK2 for now. It's okay as long as we don't have to write a different test code for WK1/WK2/iOS/macOS.
Comment on attachment 331385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331385&action=review > LayoutTests/resources/ui-helper.js:16 > + testRunner.runUIScript(` This assumes WebKitTestRunner. We should make this compatible with DumpRenderTree as well.
I'm resetting the bug assignment as I don't plan to work on this in the short term. I started making basic-gestures.js more consistent with UIHelper (use of promises) in bug 181798 and bug 181841. I also modified one scrolling test I wrote in the past (bug 182041) to use functions from basic-gestures.js ; and the same for new tests I'm introducing in bug 173833. If someone is interested in unifying / generalize the use of uiController.dragFromPointToPoint via the UIHelper class, feel free to take that bug.