Bug 181841

Created attachment 331715 [details] Patch

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

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

The failure on mac seems unrelated as it happened without the patch: https://webkit-queues.webkit.org/patch/331715/mac-ews

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 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 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); ```

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.

<rdar://problem/36660038>