Description: When a video is finished playing the slider overlaps the time length before reseting to the beginning. Prerequisite: Have a video on the DUT Repro Steps: 1) Update a Flame to 20140613000203 2) Open Video app 3) Select a video and play Actual: The slider overlaps the video time length Expected: The slider does not overlap the time Environmental Variables: Device: Flame Build ID: 20140613000203 Gaia: a3a5322692578e0a577fb7fa08e32144b2b05ba3 Gecko: 8897bc43f59b Version: 32.0a2 (2.0) Firmware Version: v121-2 User Agent: Mozilla/5.0 (Mobile;rv:32.0) Gecko/32.0 FIrefox/32.0 Notes: Repro frequency: 100% See attached: screenshot, logcat

Issue does NOT repro on 1.4 Flame v1.4 Environmental Variables Device: Flame v1.4 BuildID: 20140613000202 Gaia: 1dae62556e642b0b2e08689e35e24e56daa8c79b Gecko: 30224c7f5e58 Version: 30.0 Firmware Version: v121-2

Qawanted to confirm and to check other devices and 2.1 Flame

Correction: Qawanted to check other devices.

This issue is reproducible on Nexus 4. The build details are below:- OS Version : 2.0.0.0-prerelease Hardware Revision : Mako Platform Version : 32.0a1 Build Identifier : 20140613193818 Git Commit Info : 2014-06-13 13:16:40 Please check the attached image.

Image showing the slider overlaps the video time when playback ends

Kindly review the PR.

Comment on attachment 8442017 [details] PR Hey Amitav! Don't forget that I'm not a developer so I do UI-reviews not code reviews - I've switched the flag. I think David might be able to do the review?, if not he'll know who can. Overall, nice catch. Do you think you can put just a little more padding between the circle and the counter? It appears to be overlapping ever so slightly still. I'm also going to include Amy from visual design to take a look. Amy, feel free to pass to Hung if you don't have time. Thanks guys!

(In reply to Tiffanie Shakespeare from comment #7) > Comment on attachment 8442017 [details] > PR > > Hey Amitav! > > Don't forget that I'm not a developer so I do UI-reviews not code reviews - > I've switched the flag. > > I think David might be able to do the review?, if not he'll know who can. > > Overall, nice catch. Do you think you can put just a little more padding > between the circle and the counter? It appears to be overlapping ever so > slightly still. I'm also going to include Amy from visual design to take a > look. > > Amy, feel free to pass to Hung if you don't have time. > > Thanks guys! Hung had worked on video player refinements so I'm not sure if this is a regression (In reply to Tiffanie Shakespeare from comment #7) > Comment on attachment 8442017 [details] > PR > > Hey Amitav! > > Don't forget that I'm not a developer so I do UI-reviews not code reviews - > I've switched the flag. > > I think David might be able to do the review?, if not he'll know who can. > > Overall, nice catch. Do you think you can put just a little more padding > between the circle and the counter? It appears to be overlapping ever so > slightly still. I'm also going to include Amy from visual design to take a > look. > > Amy, feel free to pass to Hung if you don't have time. > > Thanks guys! Hung had worked on video player refinements so I'm not sure why the player is showing up like this.

The video player code is actually a duplicate of the music player. I don't see this happening in music so can we ensure the scrubber has the same code also? Thanks

(In reply to Josh Schmitt from comment #3) > Correction: > > Qawanted to check other devices. This issue does reproduce on Flame 2.1 and Buri 2.0. Following STR from Comment 0, the slider overlaps the video time length. Environmental Variables: Device: Flame Master Build ID: 20140627040205 Gaia: b8f36518696f3191a56e4f33447ee9d6ec820da1 Gecko: 9290d7995f98 Version: 33.0a1 (Master) Firmware Version: v122 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Environmental Variables: Device: Buri 2.0 Build ID: 20140627000201 Gaia: 8df02268fcd7e80c5fab8c1ec099772e37f8659d Gecko: 731a5e8831e6 Version: 32.0a2 (2.0) Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

regression but not nomming; seems like a polish-issue. Nice to have but not urgent.

Sorry I dropped the ball here. Russ, would you take a look at the attached patch and see if it fixes the problem and looks good to you? (And whether it fixes the problem in both versions of the video player?)

Amitav, Thanks for this patch. I've asked Russ to review it. Please note that in comment 7, our UX designer has asked you to make some tweaks. Also, keep in mind that the video app has a separate set of code for playing videos by the view activity, so you might want to check whether a fix is needed for that case as well (or verify that this CSS is shared by both players)

Comment on attachment 8442017 [details] PR Regarding whether the patch addresses both use cases, playing videos from the video app and playing videos from other apps (e.g., as an email attachment), the answer is yes, that patch addresses both use cases. In addition, I will reiterate what Tiffanie mentioned in comment #7, that the slider still overlaps very slightly with the duration text. I'm giving r+ with the assumption the css will be tweaked to address the (small) spacing issue that remains.

Hi Russ, I modified the existing PR to avoid the below things. 1.Slight overlap 2.Play Head is moving outof progress bar. So,please review the changes.If the patch is fine i will upload the PR for the same. Thanks.. Sireesha

Since Sireesha had got r+ and had a great work on this, I change the assignee to her. Thanks Sireesha.

Can you please post a pull request so we can get a test run prior to committing? Thanks!

I will post PR soon. Thanks.. Sireesha (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #18) > Can you please post a pull request so we can get a test run prior to > committing? Thanks!

PR is created for this issue.

This is a generic bug, shouldn't be 2.0M+ but 2.0+.

MClemmons or No-Jun, Could you please test this patch out? Thanks Hema

I verified the patch on master branch, and it looks good, works fine with portrait/landscape mode on short/long videos.

Russ, I'm assigning this to you to land on master and request uplift to 2.1 and 2.0. Please don't land Sireesha's PR directly, however because it changes the executable bit of the CSS file (which means that Sireesha uses Windows, I think). Also, you might want to verify that the extra spacing that Tif requested in comment 7 has been put in place.

The duplicate bug (bug 1043702) has a patch that is a bit cleaner than the PR for this bug. I have added some comments to the css of the bug 1043702 patch and have created a new PR [1]. I will land this new PR when the try server run is green. [1] https://github.com/mozilla-b2g/gaia/pull/24905

Master: https://github.com/mozilla-b2g/gaia/commit/96c1272cd2623616b8fa77237cc48cb82951d953

Comment on attachment 8496815 [details] Pointer to Pull Request.html NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: [Testing completed]: [Risk to taking this patch] (and alternatives if risky): [String changes made]:

Comment on attachment 8496815 [details] Pointer to Pull Request.html NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): As far as I can tell, bug has existed for some time. [User impact] if declined: Bad user experience, makes app look unprofessional. [Testing completed]: Manual testing [Risk to taking this patch] (and alternatives if risky): Low risk, changes were to a single video css file. Changes do not affect translatable text so there is no danger the patch will affect when language is non-english. [String changes made]: None

hi, would you please help to confirm if this patch checked in woodduck branch? Thank you very much!

Hi Woodduck, The patch approval for landing on 2.0 is request for approval by Russ. Once it approved. the fix will land on 2.0 and merge to 2.0M. You will have the fix then and should be before next Friday.

(In reply to Josh Cheng from comment #31) > Hi Woodduck, > The patch approval for landing on 2.0 is request for approval by Russ. Once > it approved. the fix will land on 2.0 and merge to 2.0M. > You will have the fix then and should be before next Friday. Hi Josh, do you mean before 10/17? As I know, our SQC will be end in this week. Thanks!

Comment on attachment 8496815 [details] Pointer to Pull Request.html looks low risk enough to uplift given this is a 2.0 regression.

v2.1: https://github.com/mozilla-b2g/gaia/commit/bc8eb493311c58f1f311a56b8b645b52bfbd2f71 v2.0: https://github.com/mozilla-b2g/gaia/commit/9791940d5093c37336dcd85f7e66e9a8445a218e

v2.0m: https://github.com/mozilla-b2g/gaia/commit/9791940d5093c37336dcd85f7e66e9a8445a218e

This issue can't repro on Flame 2.0, 2.1, 2.2 See attachment: Verify_screenshot.png Reproducing rate: 0/5 Woodduck2.0 build: Gaia-Rev 60146ec47cd38a8be8ed22e0116902eceb9ac067 Gecko-Rev cdfbe9866cf0b71b33454926638ce0cd8bb1fb00 Build-ID 20141117050313 Version 32.0 Flame 2.1 build: Gaia-Rev 81160ad79e5b4c21967418dd63f1a1d08d77924e Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/3572aa3e6766 Build-ID 20141117001201 Version 34.0 Flame 2.2 build: Gaia-Rev ae3a84acaab80a5b35d5542d63e68462273c8a1b Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/2d0a51ef828d Build-ID 20141117160200 Version 36.0a

This issue can't repro on Woodduck2.0 See attachment: Verify_woodduck.png Reproducing rate: 0/5 Woodduck 2.0 biuld: Gaia-Rev cc690f8016b672475dc186bc7fd58aef45e684b7 Gecko-Rev 03d3ab62d5b07b915434f2d1d68495ad5915ecd2 Build-ID 20141118184148 Version 32.0