561108 - Add-on install blocked notification is broken

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Brian King [:kinger]]

After building a debug addonsmgr branch, add-ons from AMO will not install.

e.g.
https://addons.mozilla.org/en-US/firefox/addon/15003

Error is:

Error: urchinTracker is not defined
Source File: https://addons.mozilla.org/js/amo2009/amo2009.min.js?64454
Line: 1

Comment 1

[Dave Townsend [:mossop]]

Do you see a notification bar to allow the install, or an install confirmation, or what?

Comment 2

[Brian King [:kinger]]

(In reply to comment #1)
> Do you see a notification bar to allow the install, or an install confirmation,
> or what?

Nothing. Just clicking the install link gives me this error and nothing happens. May be an AMO bug?

Comment 3

[Dave Townsend [:mossop]]

Combination of two things here. Bug 553092 means that the permissions for AMO to automatically install aren't set yet, you can fix that by just looking at the permissions in the preferences window first. And this bug is that the notification bar isn't displaying for blocked sites anymore, which is strange since we have tests for that...

Comment 4

[Dave Townsend [:mossop]]

Fallout from removing the xpinstall.enabled pref.
Fixed in http://hg.mozilla.org/projects/addonsmgr/rev/a59babfd02f0

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can we please check what's wrong with the automated tests and why they haven't failed?

Comment 6

[Dave Townsend [:mossop]]

The automated tests we have only test that the API sends the correct notifications, they don't test that Firefox does the right thing because when we did it broke on SeaMonkey etc. We need to add some additional tests to test the Firefox side of this.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

An option for automated tests could be Mozmill tests which I will have to rewrite in any case. I'll flag in-litmus too for at least one manual test.

Comment 8

[Brian King [:kinger]]

(In reply to comment #4)
> Fallout from removing the xpinstall.enabled pref.
> Fixed in http://hg.mozilla.org/projects/addonsmgr/rev/a59babfd02f0

Confirmed fixed.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

We're not marking these as fixed until they're landed on trunk - instead, add [fixed-in-addonsmgr] to the whiteboard.

Comment 10

[Dave Townsend [:mossop]]

rs=sdwilsh over IRC

Comment 11

[Dave Townsend [:mossop]]

Landed as a part of http://hg.mozilla.org/mozilla-central/rev/704046d2ddea

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave, for the manual test I would need example steps. The extension Brian mentioned in comment 0 gets installed successfully now. So it's no longer a valid test case.

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> Dave, for the manual test I would need example steps. The extension Brian
> mentioned in comment 0 gets installed successfully now. So it's no longer a
> valid test case.

If you remove AMO from the whitelist then any add-on on AMO becomes a valid test. The notification bar should appear and clicking allow should bring up the install dialog.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100518 Minefield/3.7a5pre ID:20100518030645

Beside the Litmus test could we also have an automated test or would such a test be able to cover even the manual part?

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Litmus test has been added:
https://litmus.mozilla.org/show_test.cgi?id=15216

Comment 16

[Dave Townsend [:mossop]]

We do now have some automated tests of the notifications in browser_bug553455.js

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #16)
> We do now have some automated tests of the notifications in
> browser_bug553455.js

Would that also make the existing Litmus test obsolete? I would think so.

Comment 18

[Dave Townsend [:mossop]]

(In reply to comment #17)
> (In reply to comment #16)
> > We do now have some automated tests of the notifications in
> > browser_bug553455.js
> 
> Would that also make the existing Litmus test obsolete? I would think so.

I would agree

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Litmus test has been disabled.