623151 - Migration assistant installation of add-ons no longer works on trunk builds

Status: RESOLVED FIXED

(Thunderbird :: Migration, defect)

Description

[Mark Banner (:standard8)]

Picked up from a tester via getsatisfaction. If you try and use the migration assistant to install the extra folder columns or compact header add-ons, then it just offers you the file to download rather than installing them.

The error console points to extensionConfigurator.onLoad being the problem as that uses Application.extensions which is no longer valid.

It also uses logException which isn't defined in that file/scope.

Comment 1

[ian wood]

I've seen this on :

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7

The assistant also gets stuck: If you click 'install addon', the next button remains greyed after the xpi is downloaded. You can go back and forward again to get around it.

Comment 2

[Jim Porter (:squib)]

Attachment: Use the new addons API

Ok, this uses the new addon manager API and also fixes the logException issue. Maybe we should have clarkbw weigh in on the UX though since 1) the "installed" message in the migrator isn't very pretty, and 2) it's non-obvious that you need to restart for this to take effect.

One option would be a final page that only shows up if you've installed an addon, which would have a short explanation and a restart button.

Comment 3

[Jim Porter (:squib)]

Actually... I guess we'd still want the addon install dialog/warning to pop up, right?

Comment 4

[Jim Porter (:squib)]

Attachment: Display the install addon dialog too

Doing it this way seems like the right thing to do, although I'm not 100% sure...

Comment 5

[Jim Porter (:squib)]

Attachment: Handle failure better

Well, after turning extension compatibility back on, I realized that my patch didn't handle failure very well, so this version fixes that.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 517227 [details] [diff] [review]
Handle failure better

This is looking good, but when the add-ons are already installed, it still offers to install the add-on - I don't think that's right.

Bryan should probably just give this a quick once-over anyway.

Comment 7

[Jim Porter (:squib)]

Attachment: Fix detection of already-installed addons

Oops, I got bitten by the "this" behavior in Javascript. This patch also counts pending installs as "already installed", too.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 519857 [details] [diff] [review]
Fix detection of already-installed addons

That's better, thanks.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 519857 [details] [diff] [review]
Fix detection of already-installed addons

This looks pretty good with some quick tests.

Comment 10

[Jim Porter (:squib)]

Adding checkin-needed. Are there litmus tests for this already?

Comment 11

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/44cd5f668b25

(Hooray, my first checkin! Hopefully I did everything right...)

Comment 12

[Mark Banner (:standard8)]

Yep, though its also useful to set the milestone to the next release version when you close the bug.

Comment 13

[Ludovic Hirlimann [:Usul]]

(In reply to comment #10)
> Adding checkin-needed. Are there litmus tests for this already?

Not that I can remember as This was superposed to be for one release only.
Adding that to my litmus todo-list

Comment 14

[Ludovic Hirlimann [:Usul]]

https://litmus.mozilla.org/show_test.cgi?id=15381