A number of media layout tests simulate clicks in the controller. These tests have to change every time we change the controller layout and we now have different layouts on different platforms, so we should have a way for tests to query the location of each element.
*** Bug 30680 has been marked as a duplicate of this bug. ***
Created attachment 92058 [details] proposed patch
Comment on attachment 92058 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=92058&action=review Did you mean to flag this for review? > LayoutTests/media/video-controls-zoomed.html:28 > + // Find the play button and click the middle of its bounding box. > + var playCoords = mediaControlsButtonCoordinates(video, "play"); > + var clickX = playCoords[0]; > + var clickY = playCoords[1]; > + clickX = clickX * 1.5; > + clickY = clickY * 1.5; > + eventSender.mouseMoveTo(clickX, clickY); Why can't you use the coordinates returned from mediaControlsButtonCoordinates() directly? > LayoutTests/media/video-test.js:252 > + if (!button) { > + failTest("Failed to find " + id + " button."); > + } You don't need the braces here. > LayoutTests/media/video-test.js:260 > + var result = new Array(); > + result[0] = x; > + result[1] = y; > + return result; You could use "return new Array(x, y)" to shorten this up slightly.
Thanks for the review Eric. I unmarked it previously because the bug this one depends on needs more work actually. (In reply to comment #3) > (From update of attachment 92058 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92058&action=review > > Did you mean to flag this for review? > > > LayoutTests/media/video-controls-zoomed.html:28 > > + // Find the play button and click the middle of its bounding box. > > + var playCoords = mediaControlsButtonCoordinates(video, "play"); > > + var clickX = playCoords[0]; > > + var clickY = playCoords[1]; > > + clickX = clickX * 1.5; > > + clickY = clickY * 1.5; > > + eventSender.mouseMoveTo(clickX, clickY); > > Why can't you use the coordinates returned from mediaControlsButtonCoordinates() directly? > Without this the test failed on GTK. Could it be a bug in the port then? I thought applying the zoom to the coordinates here would be ok. > > LayoutTests/media/video-test.js:252 > > + if (!button) { > > + failTest("Failed to find " + id + " button."); > > + } > > You don't need the braces here. > > > LayoutTests/media/video-test.js:260 > > + var result = new Array(); > > + result[0] = x; > > + result[1] = y; > > + return result; > > You could use "return new Array(x, y)" to shorten this up slightly. Right, will do for next iteration!
(In reply to comment #4) > > > LayoutTests/media/video-controls-zoomed.html:28 > > > + // Find the play button and click the middle of its bounding box. > > > + var playCoords = mediaControlsButtonCoordinates(video, "play"); > > > + var clickX = playCoords[0]; > > > + var clickY = playCoords[1]; > > > + clickX = clickX * 1.5; > > > + clickY = clickY * 1.5; > > > + eventSender.mouseMoveTo(clickX, clickY); > > > > Why can't you use the coordinates returned from mediaControlsButtonCoordinates() directly? > > > > Without this the test failed on GTK. Could it be a bug in the port then? I thought applying the zoom to the coordinates here would be ok. > Oh, I guess it make sense that the coordinates returned are untransformed. A comment in the test explaining why you are transforming them here would be helpful.
Created attachment 92405 [details] proposed patch
Created attachment 92407 [details] proposed patch
Created attachment 92408 [details] proposed patch
Comment on attachment 92408 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=92408&action=review Excellent work! > LayoutTests/media/video-test.js:248 > + var button; > + var controlsShadow = layoutTestController.shadowRoot(element).firstChild.firstChild; > + for (child = controlsShadow.firstChild; child; child = child.nextSibling) { > + if (layoutTestController.shadowPseudoId(child) == "-webkit-media-controls-" + id) { > + button = child; > + break; > + } > + } It's not possible to do something like getElementById on the shadow root?
I can land this after patch for bug 60034 is in the tree.
Committed r85934: <http://trac.webkit.org/changeset/85934>
http://trac.webkit.org/changeset/85934 might have broken Windows 7 Release (Tests) The following tests are not passing: media/video-controls-visible-audio-only.html
(In reply to comment #12) > http://trac.webkit.org/changeset/85934 might have broken Windows 7 Release (Tests) > The following tests are not passing: > media/video-controls-visible-audio-only.html See bug 59081 Will skip this test on win.