Closed Bug 996122 Opened 6 years ago Closed 6 years ago

Play button superimposed on videos at http://www.mozilla.org/en-US/foundation/annualreport/2012/

Categories

(Toolkit :: Video/Audio Controls, defect)

31 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox31 + verified

People

(Reporter: alex_mayorga, Assigned: DChen)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

STR:
- Load http://www.mozilla.org/en-US/foundation/annualreport/2012/
- Scroll down to "MITCHELL BAKER" video
- Click the blue placeholder so the video plays

Result:
There's a play button superimposed on the video while it plays.

Expected result:
There's no play button superimposed on the video while it plays.

Configuration:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140414030203 CSet: 215080b813a7

Confirmations:
"Confirmed. No dogfooding going on at Mozilla? ](*,)

The loading-spinner for the first few seconds in the unaffected version ain't too pretty either." Elbart http://forums.mozillazine.org/viewtopic.php?p=13469081#p13469081

"Confirmed on Nightly31.0a1 but not on Aurora30.0a2." Alice0775 http://forums.mozillazine.org/viewtopic.php?p=13469093#p13469093
Regression-range:

m-c:
Last good revision: 1417d180a1d8 (2014-04-01)
First bad revision: 4941a2ac0786 (2014-04-02)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1417d180a1d8&tochange=4941a2ac0786

m-i:
Last good revision: 0623b2fe08c8
First bad revision: 65338f03492b
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0623b2fe08c8&tochange=65338f03492b

Suspected:
c23e2ff6c65e	Danny Chen — Bug 729111 - Redisplay centered play video button after video is resized. r=jaws
Blocks: 729111
Component: Untriaged → Video/Audio Controls
Keywords: regression
Product: Firefox → Toolkit
Version: Trunk → 31 Branch
Assignee: nobody → dannychen210
Status: NEW → ASSIGNED
Attached patch Patch 1Splinter Review
Attachment #8408664 - Flags: review?(jaws)
Comment on attachment 8408664 [details] [diff] [review]
Patch 1

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

This didn't fix the issue with the pop-up videos at http://www.mozilla.org/en-US/foundation/annualreport/2012/. Did it work for you?
Attachment #8408664 - Flags: review?(jaws) → review-
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)

> This didn't fix the issue with the pop-up videos at
> http://www.mozilla.org/en-US/foundation/annualreport/2012/. Did it work for
> you?

Is it exactly the same on your end? The button goes away when I try the videos.
Comment on attachment 8408664 [details] [diff] [review]
Patch 1

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

I'm really sorry, when I applied the patch earlier it had some conflicts (due to your other patch that just landed). I updated the file but forgot to hit save before compiling and testing.

This patch does indeed fix the issue. Thanks!
Attachment #8408664 - Flags: review- → review+
Comment on attachment 8408664 [details] [diff] [review]
Patch 1

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

::: toolkit/content/widgets/videocontrols.xml
@@ +1392,5 @@
>  
>                      if ((this._overlayPlayButtonHeight + this._controlBarHeight) > videoHeight || this._overlayPlayButtonWidth > videoWidth)
>                          this.clickToPlay.hidden = true;
> +                    else if (this.clickToPlay.hidden && !this.video.played.length && this.video.paused)
> +                        // Include this.video.paused to handle when a video is playing but hasn't processed any frames yet

By the way, please add braces to the 'if' and 'else if' conditions, and wrap this comment to 80-lines. These two statements are pretty long now, and I want to make sure that subtle bugs don't get introduced.
s/80-lines/80-characters/ but you probably figured that ;)
I went ahead and made the changes for you and pushed them to the fx-team repo,
https://hg.mozilla.org/integration/fx-team/rev/d0849463abf6
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d0849463abf6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla31
QA Whiteboard: [good first verify]
Flags: in-testsuite?
Works/fixed/resolved

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
BuildID 20140714151536

[bugday-20140716]
QA Whiteboard: [good first verify] → [good first verify][bugday-20140716]
You need to log in before you can comment on or make changes to this bug.