Closed Bug 1167999 Opened 10 years ago Closed 10 years ago

Give video documents a background-color that's similar to darknoise image

Categories

(Toolkit :: Themes, defect)

Product:
Toolkit
Component:
Themes
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla41
Project Flags:
blocking-b2g 2.2+
Tracking Flags:
Tracking Status
firefox39 --- wontfix
firefox40 --- wontfix
firefox41 --- fixed
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: dholbert, Assigned: dholbert)

References

(
URL
)

Details

(Whiteboard: [caf priority: p2][CR 811812])

Attachments

(4 files)

fix v1
1.60 KB, patch
seth
: review+
jocheng
: approval-mozilla-b2g37+
Details | Diff | Splinter Review
logcat_0826.txt
80.41 KB, text/plain
Details
0826.mp4
2.17 MB, video/mp4
Details
1422.mp4
1.94 MB, video/mp4
Details
As discussed in bug 1155424 comment 39, we apparently mis-position the viewport for video documents when coming out of full-screen mode, which means we end up briefly painting a strip of white at the bottom of the screen. (and this is a bit jarring) I also occasionally see a bit of white at the bottom of my Flame if I try to overscroll a video document. This seems to be fixed if I add an explicit background-color to the <body> (in addition to the background-image). This color fills in the white areas discussed above, making them inperceptible. Hence, filing this bug on adding a background-color, to back up the background-image.
Attached patch fix v1 — — Splinter Review
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
(In reply to Daniel Holbert [:dholbert] from comment #0) > making them inperceptible. er, "imperceptible". [CC seth & jaws who have each looked at the top-level image/video document styling in the past, in case they have thoughts on this bug. Not sure who to tag for review; I might tag seth, after I've tested this on b2g a bit more without any other tweaks, to be sure it really helps]
Comment on attachment 8609911 [details] [diff] [review] fix v1 Seth, does this seem reasonable? I tested a more obvious background-color (lime), with both of the use-cases in comment 0 (coming out of full-screen, & overscrolling) -- and confirmed that the specified background-color does indeed show up at the bottom of my phone in both cases. So, it's nice to have the background color be basically the same as the background image. (The fact that this patch helps may be a real bug... But for now, I think we're really just looking for a quick & safe CSS fix to prevent the white bar in bug 1155424, which we can backport to 2.2 before it ships, and I think this fits that requirement.)
Attachment #8609911 - Flags: review?(seth)
Comment on attachment 8609911 [details] [diff] [review] fix v1 Review of attachment 8609911 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. (It'd obviously be nice to fix the real bug at some point, but this should remove the time pressure.)
Attachment #8609911 - Flags: review?(seth) → review+
Thanks! (bug 1168022 tracks fixing the "real bug", I think, for the leaving-fullscreen use case [not sure about the overscroll use case])
Comment on attachment 8609911 [details] [diff] [review] fix v1 Requesting approval‑mozilla‑b2g37. Note that (from my testing at least) this fixes bug 1155424, which is marked "blocking-b2g: 2.2+". [Approval Request Comment] Bug caused by (feature/regressing bug #): Unknown User impact if declined: White flash at bottom of screen when exiting full-screen video. Testing completed: Local testing, on Flame device, w/ opt debug build. Risk to taking this patch (and alternatives if risky): Low risk; just providing a background-color behind a background-image, as a fallback. String or UUID changes made by this patch: None.
Attachment #8609911 - Flags: approval-mozilla-b2g37?
blocking a blocker
blocking-b2g: --- → 2.2+
NI Josh for b2g37 approving.
Flags: needinfo?(jocheng)
Flags: needinfo?(jocheng)
Attachment #8609911 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Thanks, Josh! (CC'ing RyanVM, as I think he's taking care of landing approved patches, from skimming recent treeherder history for mozilla-b2g37. Please someone let me know if that's wrong & if I should proceed & land it myself.)
https://hg.mozilla.org/mozilla-central/rev/d2069e21f34a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(In reply to Daniel Holbert [:dholbert] from comment #10) > Thanks, Josh! (CC'ing RyanVM, as I think he's taking care of landing > approved patches, from skimming recent treeherder history for mozilla-b2g37. > Please someone let me know if that's wrong & if I should proceed & land it > myself.) We have bug queries for such things :P https://wiki.mozilla.org/Release_Management/B2G_Landing#Automatic_Branch_Uplifts
(Heh, of course -- I didn't mean to say *you* skim treeherder. I meant, from my own skimming of treeherder history for mozilla-b2g37, it looks like you're taking care of landings. Anyway: thanks in advance for uplifting!)
Do we care about this for Aurora/Beta?
status-b2g-master: --- → fixed
status-firefox39: --- → ?
status-firefox40: --- → ?
Flags: needinfo?(dholbert)
This patch changes theme colours for OSX and Windows, does this affect b2g? If so, which theme is b2g using for reference?
b2g (and Linux as well, I think) uses the windows theme CSS file in the attached patch. (for historical reasons, presumably)
Flags: needinfo?(dholbert)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #15) > Do we care about this for Aurora/Beta? I don't think so; I think this can just ride the trains for Desktop/Android. For Desktop, this patch has no user-visible effect, so clearly no need to backport there. For Android, we have a bug similar to the b2g one -- bug 1168613 -- but it's worse in that it's not just a "flash", but a persistent-until-you-touch-the-screen issue. So this doesn't help as much for the Android issue (hence, less benefit from backporting). And users will get this fix (and hopefully an actual fix for bug 1168613) before too long, anyway, since Android is on 6-week trains like Desktop.
Whiteboard: [CR 811812]
Whiteboard: [CR 811812] → [caf priority: p2][CR 811812]
Attached file logcat_0826.txt —
This bug has been verified fail on latest Nightly build of Flame v2.2&v3.0 STR: 1.Open browser 2.Go to rtsp://203.35.158.135/TRL/trl-upload/Highbit/mpeg4_aac_114k.3gp 3.Click on fullscreen button 4.Click on window mode button to switch back to windowed mode Actual result: You'll see a white bar flash at the bottom of the screen after coming out of full screen. Found time:08:26 See video:0826.mp4 See log:logcat_0826.txt Reproduce rate:4/5 Device:Flame 2.2(fail) Build ID 20150602162502 Gaia Revision a9aeb08263f1a727136e8ae78425e52431c82770 Gaia Date 2015-06-02 13:04:40 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5b3f1796ddf6 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150602.195401 Firmware Date Tue Jun 2 19:54:11 EDT 2015 Bootloader L1TC000118D0 Device:Flame 3.0(fail) Build ID 20150602160205 Gaia Revision 6d477a7884273886605049b20f60af5c1583a150 Gaia Date 2015-06-01 16:41:42 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/9eae3880b132 Gecko Version 41.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150602.192511 Firmware Date Tue Jun 2 19:25:20 EDT 2015 Bootloader L1TC000118D0
Attached video 0826.mp4 —
QA Whiteboard: [MGSEI-Triage+]
Flags: needinfo?(echang)
So comment 20 (and attatched video) seems to be saying the white bar is still visible, which is bad. (This is odd, because I definitely can't reproduce in a flame with latest Firefox OS 3.0 nightly.)
(In reply to Daniel Holbert [:dholbert] from comment #22) > (This is odd, because I definitely can't reproduce in a flame with latest > Firefox OS 3.0 nightly.) Correction: I can't reproduce *when using an ogg video* like e.g. http://media.tinyvid.tv/1pjr8ctdx0bpy.ogg (which was the testcase that I used when fixing this). I can, however, reproduce the white bar using the RTSP url mentioned in comment 20. (and bug 1155424 comment 0) I filed this bug purely to cover making a targeted change, and that change has stuck, though it didn't fix all of the symptoms that we hoped it would. So, rather than tracking comment 20 etc. on this bug, I reopened bug 1155424; let's leave this bug here closed & do further investigation on bug 1155424 (or on further helper-bugs).
(For verification purposes on this bug here, please use an ogg video like http://media.tinyvid.tv/1pjr8ctdx0bpy.ogg . That's what I used when testing / diagnosing this bug, and from my testing, the patch that landed did fix the white bar with those sorts of videos.)
URL: http://media.tinyvid.tv/1pjr8ctdx0bpy...
I cannot reproduce the issue that with link (2); And with link (1) I cannot even play that from fxos, android or desktop browser, not able to check with link 1. (1) rtsp://203.35.158.135/TRL/trl-upload/Highbit/mpeg4_aac_114k.3gp (2) http://media.tinyvid.tv/1pjr8ctdx0bpy.ogg
Flags: needinfo?(echang) → needinfo?(zikui.yang)
Attached video 1422.mp4 —
Per comment 24 This bug has been verified as pass on latest build of Flame v2.2 & v3.0 and Nexus5 v2.2&v3.0. STR: 1.Open browser 2.Go to http://media.tinyvid.tv/1pjr8ctdx0bpy.ogg 3.Click on fullscreen button 4.Click on window mode button to switch back to windowed mode Actually result : You'll not see a white bar flash at the bottom of the screen after coming out of full screen. See video:1422.mp4 Reproduce rate: 0/30 Device Flame 2.2(pass): Build ID 20150603162502 Gaia Revision b92e782ca12397acc3eb52f2e237522c0213f4e0 Gaia Date 2015-06-03 21:02:56 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/74421c778aff Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150603.195036 Firmware Date Wed Jun 3 19:50:46 EDT 2015 Bootloader L1TC000118D0 Device Flame 3.0(pass): Build ID 20150603160203 Gaia Revision 45dc6306cf502a4f00ae9f8bd8293a8a3a37c07b Gaia Date 2015-06-03 17:32:50 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/0920f2325a6d Gecko Version 41.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150603.192042 Firmware Date Wed Jun 3 19:20:53 EDT 2015 Bootloader L1TC000118D0 Device Nexus5 2.2(pass): Build ID 20150603162502 Gaia Revision b92e782ca12397acc3eb52f2e237522c0213f4e0 Gaia Date 2015-06-03 21:02:56 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/74421c778aff Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150603.194836 Firmware Date Wed Jun 3 19:48:52 EDT 2015 Bootloader HHZ12f Device Nexus5 3.0(pass): Build ID 20150603160203 Gaia Revision 45dc6306cf502a4f00ae9f8bd8293a8a3a37c07b Gaia Date 2015-06-03 17:32:50 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/0920f2325a6d Gecko Version 41.0a1 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150603.193140 Firmware Date Wed Jun 3 19:31:59 EDT 2015 Bootloader HHZ12f
Flags: needinfo?(zikui.yang)
Thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: