915499 - Defect - Download progress quickly flashes as "completed" and then starts the download progress

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Downloads, defect, P2)

Description

[Kamil Jozwiak [:kjozwiak]]

Sometimes when you wait about 2 seconds before selecting "Save" during a download, you'll notice that the entire download progress will appear blue and then quickly start displaying the correct progress. This appears as a quick "flash".

This usually happens on smaller downloads like the VLC executable. Reproduced this pretty consistently.

Steps to reproduce the issue:

1) Open Firefox Metro
2) Go to http://get.videolan.org/vlc/2.0.8/win32/vlc-2.0.8-win32.exe
3) The download will start counting down and the download app bar should slide into view from the bottom
4) Once the download app bar slides into view with the "Save" option, wait 2 seconds and then select "Save". Notice the the download progress circle will flash completely blue and then return to the correct progress

Current Behavior:

- The download progress indicator quickly flashes blue and appears as the download was completed and then quickly starts displaying the correct progress

Expected Behavior:

- The download progress circle should appear "fully" blue as this makes it seem as the download was completed and then quickly starts showing the correct progress

Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-09-11-03-02-58-mozilla-central/

Comment 1

[Sam Foster [:sfoster] (he/him)]

Attachment: Fix image loading/rendering logic in circular-progress-indicator binding

We had a race condition in updateProgress while the image was still loading. This patch rearranges that logic so the canvas operations only take place when that image has completed loading. Also catches a corner case where we might reset while the image is loading. 

updateProgress method still returns [startAngle, endAngle] whether it renders or not.

Comment 2

[Rodrigo Silveira [:rsilveira] [:rodms]]

Comment on attachment 8345495 [details] [diff] [review]
Fix image loading/rendering logic in circular-progress-indicator binding

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

looks good.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Landed on fx-team: 
https://hg.mozilla.org/integration/fx-team/rev/9c4075d7e10c

also, adding [next-aurora-uplift] to whiteboard for uplift batching.

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9c4075d7e10c

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8345495 [details] [diff] [review]
Fix image loading/rendering logic in circular-progress-indicator binding

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown

User impact if declined: polish issue in the initial display of the download progress indicator in Metro.

Testing completed (on m-c, etc.): Landed on m-c on 12/11.

Risk to taking this patch (and alternatives if risky): Very low-risk Metro-only patch.

String or IDL/UUID changes made by this patch: None.

Comment 6

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/27a40ec39d19

Comment 7

[Kamil Jozwiak [:kjozwiak]]

Went through the following defect for iteration #20 without any issues. Used the following builds:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-12-28-03-02-04-mozilla-central/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-12-29-00-40-02-mozilla-aurora/

- Went through the original test case from comment #0 without any issues
- Downloaded several small files and made sure that the race condition didn't occur this time around
- Downloaded several large files and made sure that the race condition didn't occur this time around
- Downloaded several different files (EXE, PDF, JPG, BMP) without the original issue occurring
- Ensured that all of the above test cases worked while in filled view
- Ensured that all of the above test cases worked while in snapped view

Comment 8

[u279076]

Setting status flags based on comment 7.