701722 - Multiple cancel prompts generated by tapping in-progress downloads several times

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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]

Attachment: 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]

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?)]

I betting Gavin didn't want to change the priority

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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]

(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]

https://hg.mozilla.org/projects/birch/rev/f5c60c4ec1ed

Comment 7

[Catalin Suciu [:csuciu]]

Verified fix on:
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111117 Firefox/11.0a1 Fennec/11.0a1