Last Comment Bug 701722 - Multiple cancel prompts generated by tapping in-progress downloads several times
: Multiple cancel prompts generated by tapping in-progress downloads several times
Status: VERIFIED FIXED
[testday-20111111]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P3 normal (vote)
: ---
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on: 702693
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-11 08:05 PST by Dave Hunt (:davehunt)
Modified: 2012-01-09 11:57 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
patch (2.31 KB, patch)
2011-11-11 16:00 PST, :Margaret Leibovic
mark.finkle: review+
Details | Diff | Review

Description Dave Hunt (:davehunt) 2011-11-11 08:05:52 PST
It's possible to tap the in-progress download multiple times by ignoring the cancel prompt. This causes multiple prompts to build up. If you never choose to cancel the download then after the download is complete and you return to Fennec you then must dismiss all accumulated prompts. If you do choose to cancel the download then the download is cancelled and the file is not opened, but you still need to dismiss all of the prompts.

Steps to reproduce:
1. Start a download for a large file (to give you time to complete these steps)
2. Bring up the Android download manager
3. Tap the in-progress download
4. Ignore the cancel prompt, and bring up the Android download manager
5. Tap the in-progress download
6. Repeat 4 & 5 as many times as you wish

Expected:
Subsequently tapping the in-progress download should focus the existing cancel prompt.

Actual:
New cancel prompts pop up on top of the existing ones. This means all must be dismissed before you can return to Fennec.
Comment 1 :Margaret Leibovic 2011-11-11 16:00:11 PST
Created attachment 573949 [details] [diff] [review]
patch

I'm somewhat confused as to how we can re-enter that JS code if it's blocked on confirmEx. This feels kind of hacky, but it fixes the problem. I'm curious if you have ideas for a better solution.
Comment 2 :Margaret Leibovic 2011-11-11 16:01:17 PST
Also, if the download finishes while the prompt is showing, the prompt becomes useless. Is there a way to dismiss it once the download finishes? Maybe we can file another bug about that.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-14 18:29:00 PST
I betting Gavin didn't want to change the priority
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-14 18:35:58 PST
Comment on attachment 573949 [details] [diff] [review]
patch

  showAlert: function dl_showAlert(aDownload, aMessage, aTitle, aIcon) { 
>     let self = this;
>+    // Use this flag to make sure we only show one prompt at a time

Add a blank line before the comment

>+    let cancelPromptShowing = false;

"cancelPrompt" is enough IMO


This patch does fix the problem, but as you suggest, it's a bit hacky. You mention that we need a way to dismiss a prompt. If we had that API, I would suggest using it to cancel an existing prompt. That way we could cancel prompts when a new prompt is needed (this bug) and cancel the prompt if the download finishes (a different bug).

It would seem a bit cleaner that way.

r+ though since this addresses the immediate issue.

File a new bug on adding a way to dismiss prompts, and we can use it to fix this bug a little better too.
Comment 5 :Margaret Leibovic 2011-11-15 10:57:44 PST
(In reply to Mark Finkle (:mfinkle) from comment #4)
> File a new bug on adding a way to dismiss prompts, and we can use it to fix
> this bug a little better too.

Filed bug 702693.
Comment 6 :Margaret Leibovic 2011-11-15 12:33:26 PST
https://hg.mozilla.org/projects/birch/rev/f5c60c4ec1ed
Comment 7 Catalin Suciu [:csuciu] 2011-11-18 04:43:24 PST
Verified fix on:
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111117 Firefox/11.0a1 Fennec/11.0a1

Note You need to log in before you can comment on or make changes to this bug.