Closed Bug 1072193 Opened 10 years ago Closed 10 years ago

Video controls don't hide if mouse leaves controls via left/right sides

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: theinternetftw, Assigned: theinternetftw)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140917194002

Steps to reproduce:

1) Move mouse onto video controls
2) Move mouse directly right or directly left until it has left the video element


Actual results:

Video controls remain visible


Expected results:

Video controls should hide after mouse leaves
(this works if the mouse leaves the controls via top or bottom)
Adding a patch for it, looks to be a simple fix.
Attachment #8494409 - Flags: review?(dolske)
First patch, also.  Hopefully I haven't misunderstood the process.
Comment on attachment 8494409 [details] [diff] [review]
patch1 - add left and right checks to onMouseInOut

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

My first thought was that this bug was crazy, and the controls clearly disappear when moving outside the video! But then I noticed you're talking about moving left/right from the actual control bar, not anywhere else on the video... Holy cow, that is indeed broken! Nice catch!

Looks like bug 757625 (which added this code) should have considered this case, but was only thinking about the specific issue there (scrubbing, not leaving the video).

This patch looks fine, but could I ask for you to do a little code cleanup while you're here? I think it would greatly improve readability, especially with the extra lines added here, to use an intermediate variable for |this.controlBar.getBoundingClientRect()|, so we don't keep repeating that.

eg:
  var controlRect = this.controlBar.getBoundingClientRect();
  if (event.clientY > controlRect.top && ....)
No problem, changes added.
Attachment #8494409 - Attachment is obsolete: true
Attachment #8494409 - Flags: review?(dolske)
Attachment #8494661 - Flags: review?(dolske)
Comment on attachment 8494661 [details] [diff] [review]
patch 2 - checkAllBoundsOnHover + code cleanup

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

Awesome! Thanks for the patch!
Attachment #8494661 - Flags: review?(dolske) → review+
Assignee: nobody → bcc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Landed in https://hg.mozilla.org/integration/fx-team/rev/36f08e5cfab0, if all goes well it should be in tomorrow's Nightly build. There will be another comment here when the code is merged and then the bug will be closed.

If you haven't already, check out https://developer.mozilla.org/en-US/docs/Introduction -- although I suspect you have, since everything here was flawless. Looking forward to your next patch! :)
https://hg.mozilla.org/mozilla-central/rev/36f08e5cfab0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: