627911 - String errors in extensions.js

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Steps to reproduce:
1. Open the add-on panel.
2. Disable your network connection.
3. Try to install an add-on.

Expected results: "Download failed" error message.

Actual results: No error message in the UI; JS errors in the console.

Bug 605494 was written before bug 518111 landed but pushed after, so it still used the old Elements.browserBundle.

Also, there is no aError argument to the addon install listener methods; it's aInstall.error.

Finally, there's a formatStringFromName that's missing it's final argument; looks like this bug has been around undetected for a long time.

Comment 1

[Wesley Johnston (:wesj)]

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

>   _createLocalAddon: function ev__createLocalAddon(aAddon) {
>-    let strings = Elements.browserBundle;
>+    let strings = Services.strings.createBundle("chrome://browser/locale/browser.properties");
> 

Can we use Strings.browser here?

Rest looks fine. We should add tests for these failures.

Comment 2

[Matt Brubeck (:mbrubeck)]

Nominating for blocking; cjones ran into this in the wild, and it's a low-risk
fix to some clearly broken corners of the add-on manager UI.
(In reply to comment #1)

> Comment on attachment 505995 [details] [diff] [review]
> Can we use Strings.browser here?

Not in a component.

> Rest looks fine. We should add tests for these failures.

I'll file a followup.

Comment 3

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/840639ac20d8

Filed bug 627925 for tests.

Comment 4

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

(In reply to comment #2)
> Nominating for blocking; cjones ran into this in the wild, and it's a low-risk
> fix to some clearly broken corners of the add-on manager UI.
> (In reply to comment #1)
> 
> > Comment on attachment 505995 [details] [diff] [review]
> > Can we use Strings.browser here?
> 
> Not in a component.

You're not in a component

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: followup

Replace Services.strings.createBundle(...) with Strings.browser (followup for the mistakenly ignored review in comment 1).

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 520323 [details] [diff] [review]
followup

Yay

Comment 7

[Matt Brubeck (:mbrubeck)]

Pushed followup: http://hg.mozilla.org/mobile-browser/rev/1cc6c0306eae

Comment 8

[Andreea Pod]

Verified on build: Mozilla /5.0 (Android;Linux armv7l;rv:5.0a2) Gecko/20110414
Firefox/5.0a2 Fennec/5.0a2 
Device: LG Optimus 2X (Android 2.2)