Closed
Bug 1005031
Opened 11 years ago
Closed 10 years ago
Video controls are displayed in the middle of the video
Categories
(Toolkit :: Video/Audio Controls, defect)
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)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
wesj
:
review+
wesj
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
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.
Reporter | ||
Comment 1•11 years ago
|
||
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
Comment 2•11 years ago
|
||
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: --- → ?
status-firefox31:
--- → affected
status-firefox32:
--- → affected
Keywords: regression,
regressionwindow-wanted
Reporter | ||
Comment 3•11 years ago
|
||
Reporter | ||
Comment 5•11 years ago
|
||
(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
Keywords: regressionwindow-wanted
Comment 6•11 years ago
|
||
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)
Updated•11 years ago
|
Component: General → Video/Audio Controls
Product: Firefox for Android → Toolkit
Comment 7•11 years ago
|
||
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)
Assignee | ||
Comment 8•11 years ago
|
||
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)
Updated•11 years ago
|
Flags: needinfo?(wjohnston)
Updated•11 years ago
|
Updated•11 years ago
|
Updated•11 years ago
|
tracking-fennec: ? → 31+
Comment 9•11 years ago
|
||
Nick - Can you assist with getting Fennec running in an emulator?
Flags: needinfo?(nalexander)
Comment 10•11 years ago
|
||
(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)
Updated•11 years ago
|
Blocks: b2g-RTSP-2.0
Comment 12•11 years ago
|
||
We should back out the patch from bug 495593 in the meantime since this bug doesn't look like it's getting any movement.
Assignee | ||
Comment 14•11 years ago
|
||
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)
Assignee | ||
Comment 15•11 years ago
|
||
Err, nevermind. I forgot I changed something else in the VM version of the patch. Is there a way to remove that?
Updated•11 years ago
|
Assignee: nobody → dannychen210
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Attachment #8425025 -
Attachment is obsolete: true
Attachment #8425025 -
Flags: feedback?(jaws)
Comment 16•10 years ago
|
||
The offending patch has been backed out of Firefox 31 now. Adjusting tracking accordingly.
tracking-firefox31:
+ → ---
Comment 17•10 years ago
|
||
FYI, this is also reproducible in FxOS: β Gaia f3b5d74dd3428c89cab06db734c62f3c9dbb8c4d β β Gecko https://hg.mozilla.org/mozilla-central/rev/e86a0d92d174 β β BuildID 20140526040203 β β Version 32.0a1
Updated•10 years ago
|
tracking-fennec: 31+ → 32+
Assignee | ||
Comment 18•10 years ago
|
||
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)
Assignee | ||
Comment 19•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8432174 -
Flags: feedback?(jaws) → feedback?(wjohnston)
Comment 20•10 years ago
|
||
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+
Updated•10 years ago
|
relnote-firefox:
--- → 32+
Updated•10 years ago
|
Keywords: reproducible
Assignee | ||
Comment 21•10 years ago
|
||
Attachment #8432174 -
Attachment is obsolete: true
Attachment #8436270 -
Flags: feedback?(wjohnston)
Comment 22•10 years ago
|
||
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 23•10 years ago
|
||
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+
Comment 24•10 years ago
|
||
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)
Reporter | ||
Updated•10 years ago
|
status-firefox33:
--- → affected
Assignee | ||
Comment 25•10 years ago
|
||
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)
Comment 26•10 years ago
|
||
mochitest-5 is the test suite that contains toolkit/content/tests/widgets
Flags: needinfo?(jaws)
Assignee | ||
Comment 27•10 years ago
|
||
Try results: https://tbpl.mozilla.org/?tree=Try&rev=e45be5899e1e
Flags: needinfo?(jaws)
Comment 28•10 years ago
|
||
Thanks! Marking checkin-needed accordingly.
Flags: needinfo?(jaws)
Keywords: checkin-needed
Comment 29•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc1c4ff7de9e
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bc1c4ff7de9e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Reporter | ||
Comment 31•10 years ago
|
||
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)
Comment 32•10 years ago
|
||
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
Comment 33•10 years ago
|
||
Comment 34•10 years ago
|
||
Please ask for (32) Beta approval on this patch.
Flags: needinfo?(dannychen210)
Assignee | ||
Comment 35•10 years ago
|
||
(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)
Comment 36•10 years ago
|
||
Details link of the patch you attached. approval-mozilla-beta flag
Flags: needinfo?(kbrosnan)
Assignee | ||
Comment 37•10 years ago
|
||
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 38•10 years ago
|
||
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+
Updated•10 years ago
|
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Reporter | ||
Comment 42•10 years ago
|
||
Verified as fixed in: Build: Firefox for Android 32 Beta 2 Device: Asus Transformer Pad TF300T (Android 4.2.1)
Comment 43•10 years ago
|
||
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
Reporter | ||
Comment 44•10 years ago
|
||
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.
Description
•