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

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Ben Craddock]

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)

Comment 1

[Ben Craddock]

Attachment: patch1 - add left and right checks to onMouseInOut

Adding a patch for it, looks to be a simple fix.

Comment 2

[Ben Craddock]

First patch, also.  Hopefully I haven't misunderstood the process.

Comment 3

[Justin Dolske [:Dolske]]

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 && ....)

Comment 4

[Ben Craddock]

Attachment: patch 2 - checkAllBoundsOnHover + code cleanup

No problem, changes added.

Comment 5

[Justin Dolske [: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!

Comment 6

[Justin Dolske [:Dolske]]

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! :)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/36f08e5cfab0