500480 - unable to install add-ons extensions

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

on both maemo and winmo (beta2 build1), I got to add-ons manager search for script, get no-script and AutoPager.  I click the install button and I get the top part of a grey box, but I cannot scroll into it.  This is the case for both of the mentioned add-ons.

Comment 1

[Al Billings [:abillings - ex-MoCo]]

I went to the extensions panel in options, searched for "script" and installed no script there with no problems.

Comment 2

[Al Billings [:abillings - ex-MoCo]]

This is on my n810 with Maemo.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.1

In fact the extension should install, the top bar seen is a progress element that should display the current download progress.
The progress display have been broken here http://hg.mozilla.org/mobile-browser/rev/5bbb4c203890 
The patch basically revert the extensions.js change done in this changeset to show how it should work.


It is not the good way to do since we already have an observer for the xpi download progress. 
Mark, you probably know this part very well, any hint?

Comment 4

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

I switched to weak references purely so I wouldn't need to un register. But since we have an ExtensionsView.uninit() and, more importantly, the code is broken - we should change back :)

Comment 5

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

(In reply to comment #3)
 
> It is not the good way to do since we already have an observer for the xpi
> download progress. 

What is "not a good way"?

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5)
> (In reply to comment #3)
> 
> > It is not the good way to do since we already have an observer for the xpi
> > download progress. 
> 
> What is "not a good way"?

I saying that cause browser.js#173 already do os.addObserver(gXPInstallObserver, "xpinstall-download-started", false); but its like gXPInstallObserver do nothing on this topic. 
Not sure why it is here?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Ok, now I see what it is for in browser.js after have read your comments on bug 437959. 
I supposed it is unused now but it can be in the future.

Comment 8

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

Ah, I see what you mean now. We really only need to handle "xpinstall-install-blocked" in browser.js for now.

Comment 9

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

Can you remove the addEventListener / removeEventListener in this patch ?

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

do you mean addInstallListener/removeInstallListenerAt?

Comment 11

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

Oops. I mean:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#173
http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#260

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch V0.2

Yes :)

Comment 13

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

https://hg.mozilla.org/mobile-browser/rev/cb8df78a156b

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

verified with beta3