Closed
Bug 829866
Opened 12 years ago
Closed 12 years ago
Play button overlay not removed from video when playing through javascript if video is display:none
Categories
(Toolkit :: Video/Audio Controls, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: an72, Assigned: tallOwen)
References
Details
(Keywords: regression, Whiteboard: [good first bug][mentor=jaws][lang=js])
Attachments
(3 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20100101 Firefox/17.0 Iceweasel/17.0.1
Build ID: 20121228193015
Steps to reproduce:
Provide a custom video play button on a page. Use javascript to display video element, and begin playback.
Actual results:
Video appears and begins playback. Browser-provided play button overlay is not removed from the video element.
Happening in Firefox 18/Windows and Firefox 17/Linux.
Expected results:
Browser-provided play button overlay is removed from the video element.
Comment 1•12 years ago
|
||
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/c07595bee6cf
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120126 Firefox/12.0a1 ID:20120126171251
Bad:
http://hg.mozilla.org/mozilla-central/rev/206305cbbeb1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120127 Firefox/12.0a1 ID:20120127022250
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c07595bee6cf&tochange=206305cbbeb1
Suspected:
8d11d8bc8091 Jared Wein — Bug 666306 - Added a large play button when the video is not autoplay and with controls enabled. r=dolske
Blocks: 666306
Status: UNCONFIRMED → NEW
Component: Untriaged → Video/Audio Controls
Ever confirmed: true
Keywords: regression
OS: Linux → All
Product: Firefox → Toolkit
Version: 18 Branch → 12 Branch
Updated•12 years ago
|
Attachment #701394 -
Attachment mime type: text/plain → text/html
Comment 2•12 years ago
|
||
Looking in to this it appears that this bug is happening because the video element is display:none right up until the "play()" function is called on the video.
The bindings are not attached to the video at this point, and so the code for the "play" event is not getting hit. To prove this, the browser debugger never hits the "play" event handling at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml#508. After the video begins playing and has subsequently been paused, calling videoElement.play() from the Web Console does hit my breakpoint and removes the play button.
A simple band-aid approach would be hide this.clickToPlay in the "timeupdate" event handler if !this.video.paused.
Hardware: x86_64 → All
Summary: Play button overlay not removed from video when playing through javascript → Play button overlay not removed from video when playing through javascript if video is display:none
Comment 3•12 years ago
|
||
I'd say setupInitialState() should take of this -- it's already responsible for handling other stuff where the video might be doing things before we've attached.
Updated•12 years ago
|
Assignee: nobody → heldtray
Status: NEW → ASSIGNED
Updated•12 years ago
|
Assignee: heldtray → owen
Whiteboard: [good first bug][mentor=jaws][lang=js]
Comment 4•12 years ago
|
||
Comment 5•12 years ago
|
||
Comment on attachment 702543 [details] [diff] [review]
Quick fix
Review of attachment 702543 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/content/widgets/videocontrols.xml
@@ +587,4 @@
> this.setPlayButtonState(false);
>
> + // Remove the play button overlay to give user an unobstructed view
> + this.clickToPlay.hidden = true;
See https://bugzilla.mozilla.org/show_bug.cgi?id=829866#c3, we should make this change in setupInitialState.
Assignee | ||
Comment 6•12 years ago
|
||
I think the issue was happening after setupInitialState. In setupInitialState, all the controls are still hidden. This patch stops the original fade in from happening in the case that the video is already playing.
Attachment #702694 -
Flags: review?(jaws)
Comment 7•12 years ago
|
||
Comment on attachment 702694 [details] [diff] [review]
Patch v0.1
Review of attachment 702694 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. Do you need me to land this for you?
Attachment #702694 -
Flags: review?(jaws) → review+
Comment 8•12 years ago
|
||
Comment 9•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•