570156 - Convert download dialog to a nsIPromptService prompt

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

We changed the styling of buttons in most dialogs and prompts but forgot the download dialog:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.xul#501

As Matt points out, we could just remove this dialog and use a prompt instead. That would reduce the browser.xul size and give the dialog the new styling.

Comment 1

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

Attachment: patch

This patch converts the dialog to a prompt. It removes the browser.xul and browser.js parts.

Comment 2

[Matt Brubeck (:mbrubeck)]

One problem is that "escape" always selects the button with index 1 (bug 345067).  This means that pressing "escape" (which includes the Back button on Android) will sometimes open the file instead of cancelling the download.

Comment 3

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

(In reply to comment #2)
> One problem is that "escape" always selects the button with index 1 (bug
> 345067).  This means that pressing "escape" (which includes the Back button on
> Android) will sometimes open the file instead of cancelling the download.

Let's assume we will fix bug 345067, if only in Fennec if it comes to that. However, I think we can get a fix into toolkit too since the prompt code is being rewritten (bug 563274).

Comment 4

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

Matt, go ahead and review this patch and we'll handle the confirmEx problem in a different bug.

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 449314 [details] [diff] [review]
patch

>+    let flags = Ci.nsIPrompt.BUTTON_POS_1_DEFAULT +

I don't think BUTTON_POS_1_DEFAULT is right, since it's either "Nothing" or "Open" depending on the dialog.  We should leave this off and we'll get button 0 ("Save") as the default.

>+helperApp.prompt=What would you like to do with:

I'm surprised the localizers haven't complained about this, since it seems like the filename wouldn't naturally come at the end of the sentence in all languages.  But it's not a new problem in this patch, and I guess they dealt with it before...

r+ with the default button change.

Comment 6

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

pushed to m-b:
http://hg.mozilla.org/mobile-browser/rev/abedeb76d366
http://hg.mozilla.org/mobile-browser/rev/4694d3d3df0a

Comment 7

[Catalin Suciu [:csuciu]]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110921
Firefox/9.0a1 Fennec/9.0a1
Device: HTC Desire
OS: Android 2.2