Add a test to confirm that the <video controls> full screen button works as intended

NEW
Unassigned

Status

()

7 years ago
6 years ago

People

(Reporter: jaws, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Created attachment 576582 [details]
Test for full-screen button

Bug 470628 added a full-screen button to the <video> controls. There was a test made with that patch, but it is getting occasional timeouts on Mac and Linux builds.

This bug is to fix up the test to figure out why the timeouts are occurring and get the test landed.
Sorry, I meant to say that the oranges are happening on Linux Opt/Debug and WinXP opt/debug.
Created attachment 576601 [details] [diff] [review]
Patch for bug 704924 - Test for full-screen button

This is the part of the previous patch that was specifically adding the new test.
Assignee: nobody → jwein
Attachment #576582 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Note: There is a potential that the timeouts were the cause of bug 718107. Knowing this now, there may be a workaround that we can do to get these tests landed.
Created attachment 642565 [details] [diff] [review]
Patch v2

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=710b15ca44f8
Attachment #576601 - Attachment is obsolete: true
Attachment #642565 - Flags: review?(dolske)
Working on fixing some of the oranges in OS X 10.7 and Win XP.

It looks like I've fixed the oranges in OSX 10.7 (https://tbpl.mozilla.org/?tree=Try&rev=d499df23938e), and I've got another version that I just pushed to tryserver to see if it will fix Windows XP (https://tbpl.mozilla.org/?tree=Try&rev=2274914b1297).
Created attachment 644308 [details] [diff] [review]
Patch v3

Ok, this patch passes on all platforms \o/

https://tbpl.mozilla.org/?tree=Try&rev=beabbe11c24b

The resize grippy on XP popup windows got in the way of clicking on the fullscreen button. Moving the pixel location of the mouse click a few pixels away fixed it.
Attachment #642565 - Attachment is obsolete: true
Attachment #642565 - Flags: review?(dolske)
Attachment #644308 - Flags: review?(dolske)
Comment on attachment 644308 [details] [diff] [review]
Patch v3

Review of attachment 644308 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/tests/widgets/test_videocontrols_fullscreen.html
@@ +75,5 @@
> +      var fullscreenButtonCenterX = win.screen.width - Math.round(fullscreenButtonWidth / 2);
> +      var fullscreenButtonCenterY = win.screen.height - Math.round(fullscreenButtonHeight / 2);
> +
> +      // Sometimes the window hasn't fully finished redrawing when the event is fired.
> +      setTimeout(function() {

*frowny face*

Would be good to get rid of this. Can you force it with a style flush? Or onpaint listener? Polling setInterval to (somehow) see if it's ready to click?
Attachment #644308 - Flags: review?(dolske) → review+
I tried doing a style flush (calling video.clientTop) but the test still timed out on OS X. <https://tbpl.mozilla.org/?tree=Try&rev=b53cd1740a35>

Since this is native anonymous content, we don't have a good way to know when the element is visible or ready to be clicked on.
Assignee: jAwS → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.