Closed Bug 1167999 Opened 9 years ago Closed 9 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: 9 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: