Closed Bug 1005031 Opened 6 years ago Closed 6 years ago

Video controls are displayed in the middle of the video

Categories

(Toolkit :: Video/Audio Controls, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla33
Tracking Status
firefox30 --- unaffected
firefox31 --- unaffected
firefox32 + verified
firefox33 --- verified
b2g-v2.0 --- verified
b2g-v2.1 --- verified
relnote-firefox --- 32+
fennec 32+ ---

People

(Reporter: CristinaM, Assigned: DChen)

References

Details

(Keywords: regression, reproducible)

Attachments

(5 files, 3 obsolete files)

Environment:
Build: Nightly (2014-05-01)
Device: Asus Transformer Pad TF300T (Android 4.2.1)

Steps to reproduce:
1. Start Fennec and go to http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm
2. Tap once on the video

Expected results:
The video control are displayed at the bottom part of the video.

Actual results:
The video controls are displayed in the middle of the video.
Regression window:
Last good revision: 7fe3ee0cf8be (2014-04-18)
First good revision: 582b2d81ebe1 (2014-04-19)

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7fe3ee0cf8be&tochange=582b2d81ebe1
Can you check on fxteam builds? http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/fx-team-android/

Screenshot?

Maybe:

d0849463abf6	Danny Chen — Bug 996122 - Play button superimposed on videos that are requested to start playing and then resize before any frames are presented. r=jaws
tracking-fennec: --- → ?
Attached image video_controls.png
I checked fxteam builds:
Good: 1397846067
Bad: 1397855367
(In reply to cristina.madaras from comment #4)
> I checked fxteam builds:
> Good: 1397846067
> Bad: 1397855367

Pushlog: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=f2e5bc417f96&tochange=d0849463abf6
Thanks, so it's either these two.

jwein@mozilla.com
Fri Apr 18 21:08:47 2014 +0000	d0849463abf6	Danny Chen — Bug 996122 - Play button superimposed on videos that are requested to start playing and then resize before any frames are presented. r=jaws

f8118ec22ed9	Danny Chen — Bug 495593 - Video controls should match scaled video size within video element. r=jaws
Flags: needinfo?(weinjared+bmo)
Component: General → Video/Audio Controls
Product: Firefox for Android → Toolkit
Most likely it's the second changeset that you have listed here.

Danny, are you able to investigate this on Android? I'm not sure if you have the hardware + build environment available.
Flags: needinfo?(weinjared+bmo) → needinfo?(dannychen210)
I don't have a recent Android device. Can Fennec be run on an emulator?

I've been building on Windows as well so I think I would need to setup a Linux VM.
Flags: needinfo?(dannychen210)
Blocks: 495593
tracking-fennec: ? → 31+
Nick - Can you assist with getting Fennec running in an emulator?
Flags: needinfo?(nalexander)
(In reply to Mark Finkle (:mfinkle) from comment #9)
> Nick - Can you assist with getting Fennec running in an emulator?

I've actually only run Fennec in an emulator (successfully, that is) with an x86 emulator.  But if that will demonstrate the issue, I can certainly help with that part.  There are two things for this:

1) building Fennec for x86 architecture; (or downloading it, if that is enough)
2) preparing the x86 emulator.

For 1), I use my regular mozconfig, a different OBJDIR, and the single additional flag

ac_add_options --target=i686-linux-android

At various times, we've had problems with debug builds so I always build optimized (do not include --disable-optimize).

For 2), I suggest our very on gbrown's excellent instructions at http://gbrownmozilla.wordpress.com/2012/11/02/running-firefox-for-android-in-the-android-x86-emulator/
Flags: needinfo?(nalexander)
Blocks: b2g-RTSP-2.0
We should back out the patch from bug 495593 in the meantime since this bug doesn't look like it's getting any movement.
Jared, indeed. Can you take care of that?
Attached patch Patch 1 (obsolete) — Splinter Review
Sorry about that. Had some difficulties getting my new build environment running.

I added '-moz-box-pack: end' to re-align the Fennec video controls to the bottom. This change doesn't appear to modify desktop Firefox as the controls overlay fills the entire height.
Attachment #8425025 - Flags: feedback?(jaws)
Err, nevermind. I forgot I changed something else in the VM version of the patch. Is there a way to remove that?
Assignee: nobody → dannychen210
Status: NEW → ASSIGNED
Attachment #8425025 - Attachment is obsolete: true
Attachment #8425025 - Flags: feedback?(jaws)
The offending patch has been backed out of Firefox 31 now. Adjusting tracking accordingly.
FYI, this is also reproducible in FxOS:

│ Gaia      f3b5d74dd3428c89cab06db734c62f3c9dbb8c4d                         │
│ Gecko     https://hg.mozilla.org/mozilla-central/rev/e86a0d92d174          │
│ BuildID   20140526040203                                                   │
│ Version   32.0a1
tracking-fennec: 31+ → 32+
Attached patch Alignment patch v1 (obsolete) — Splinter Review
The cause was touch controls being oriented horizontally within controlsOverlay while the desktop version is vertical.

I have a separate patch for adjusting Bug 495593 to Android. Not sure where it would go.
Attachment #8432168 - Flags: feedback?(jaws)
Attached patch Alignment patch v2 (obsolete) — Splinter Review
Missed a bracket while trying to split the patches into two.
Attachment #8432168 - Attachment is obsolete: true
Attachment #8432168 - Flags: feedback?(jaws)
Attachment #8432174 - Flags: feedback?(jaws)
Comment on attachment 8432174 [details] [diff] [review]
Alignment patch v2

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

I'd probably add a comment explaining why this is needed since its not obvious from the other styles in this file.
Attachment #8432174 - Flags: feedback?(wjohnston) → feedback+
Keywords: reproducible
Attachment #8432174 - Attachment is obsolete: true
Attachment #8436270 - Flags: feedback?(wjohnston)
Comment on attachment 8436270 [details] [diff] [review]
Patch v2 with comments

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

Looks good. Thanks :)
Attachment #8436270 - Flags: feedback?(wjohnston) → feedback+
Comment on attachment 8436270 [details] [diff] [review]
Patch v2 with comments

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

I'm fine r+'ing this too.
Attachment #8436270 - Flags: review+
Danny, do you have access to tryserver? We should push this patch to tryserver before landing it. If you don't already have access, then let's get you access :)

You can file a bug similar to https://bugzilla.mozilla.org/show_bug.cgi?id=664170 to get tryserver access. See https://www.mozilla.org/hacking/commit-access-policy/ for more info.
Flags: needinfo?(wjohnston) → needinfo?(dannychen210)
I have level 1 commit access now. Which test suites should I target for this component under http://trychooser.pub.build.mozilla.org/ ?
Flags: needinfo?(dannychen210) → needinfo?(jaws)
mochitest-5 is the test suite that contains toolkit/content/tests/widgets
Flags: needinfo?(jaws)
Thanks! Marking checkin-needed accordingly.
Flags: needinfo?(jaws)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bc1c4ff7de9e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Verified as fixed in:
Build: Firefox for Android 33.0a2 (2014-06-30)
Device: Asus Transformer Pad TF300T (Android 4.2.1) and Nexus 4 (Android 4.4.2)
Thanks everyone!
Verified it on FxOS V2.1.
Attach the screenshot (2014-07-04-20-23-49.png)

* Build information:
 - Gaia      b597b86274ab109d7ef530bc0c4b6adccddae4e4
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/e8df6826a571
 - BuildID   20140704040205
 - Version   33.0a1
Please ask for (32) Beta approval on this patch.
Flags: needinfo?(dannychen210)
(In reply to Kevin Brosnan [:kbrosnan] from comment #34)
> Please ask for (32) Beta approval on this patch.

How do I do that?
Flags: needinfo?(dannychen210) → needinfo?(kbrosnan)
Details link of the patch you attached. approval-mozilla-beta flag
Flags: needinfo?(kbrosnan)
Comment on attachment 8443017 [details] [diff] [review]
Patch v2 with comments and +r in commit message

Approval Request Comment
[Feature/regressing bug #]: patch to fix regression caused by bug 495593
[User impact if declined]: none, bug 495593 has been backed out
[Describe test coverage new/current, TBPL]: using current mochitest-5 suite
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8443017 - Flags: approval-mozilla-beta?
Comment on attachment 8443017 [details] [diff] [review]
Patch v2 with comments and +r in commit message

(In reply to Danny Chen [:DChen] from comment #37)
> [Feature/regressing bug #]: patch to fix regression caused by bug 495593

As stated below, bug 495593 was backed out. Is this really the source of the regression?

> [User impact if declined]: none, bug 495593 has been backed out

The user impact is that video controls will display in the middle of the video instead of at the bottom.

Beta+
Attachment #8443017 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I believe the backout was for 31. I can reproduce this issue in 32.
Verified as fixed in:
Build: Firefox for Android 32 Beta 2
Device: Asus Transformer Pad TF300T (Android 4.2.1)
Attached image Verify_image.png
This issue has been verified successfully on Flame 2.0,2.1

See attachment: Verify_image.png
Reproducing rate: 0/5
Flame 2.1 build:
Gaia-Rev        db2e84860f5a7cc334464618c6ea9e92ff82e9dd
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119
Build-ID        20141126001202
Version         34.0
FLame 2.0 build:
Gaia-Rev        f9d6e3d83c3922e9399a6c27f5ce4cdd27bdfd05
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/45112935086f
Build-ID        20141126000203
Version         32.0
I will mark this as Verified fixed based on comment 43.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.