Closed Bug 623151 Opened 10 years ago Closed 9 years ago

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

Categories

(Thunderbird :: Migration, defect)

defect
Not set
normal

Tracking

(blocking-thunderbird5.0 needed)

RESOLVED FIXED
Thunderbird 5.0b1
Tracking Status
blocking-thunderbird5.0 --- needed

People

(Reporter: standard8, Assigned: squib)

References

()

Details

(Whiteboard: [gs])

Attachments

(1 file, 3 obsolete files)

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.
blocking-thunderbird5.0: --- → needed
Whiteboard: [gs]
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.
Attached patch Use the new addons API (obsolete) — Splinter Review
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.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #514740 - Flags: review?(bugzilla)
Actually... I guess we'd still want the addon install dialog/warning to pop up, right?
Doing it this way seems like the right thing to do, although I'm not 100% sure...
Attachment #514740 - Attachment is obsolete: true
Attachment #514745 - Flags: review?(bugzilla)
Attachment #514740 - Flags: review?(bugzilla)
Attached patch Handle failure better (obsolete) — Splinter Review
Well, after turning extension compatibility back on, I realized that my patch didn't handle failure very well, so this version fixes that.
Attachment #514745 - Attachment is obsolete: true
Attachment #517227 - Flags: review?(bugzilla)
Attachment #514745 - Flags: review?(bugzilla)
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.
Attachment #517227 - Flags: ui-review?(clarkbw)
Attachment #517227 - Flags: review?(bugzilla)
Attachment #517227 - Flags: review-
Oops, I got bitten by the "this" behavior in Javascript. This patch also counts pending installs as "already installed", too.
Attachment #517227 - Attachment is obsolete: true
Attachment #519857 - Flags: ui-review?(clarkbw)
Attachment #519857 - Flags: review?(bugzilla)
Attachment #517227 - Flags: ui-review?(clarkbw)
Comment on attachment 519857 [details] [diff] [review]
Fix detection of already-installed addons

That's better, thanks.
Attachment #519857 - Flags: review?(bugzilla) → review+
Comment on attachment 519857 [details] [diff] [review]
Fix detection of already-installed addons

This looks pretty good with some quick tests.
Attachment #519857 - Flags: ui-review?(clarkbw) → ui-review+
Adding checkin-needed. Are there litmus tests for this already?
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/44cd5f668b25

(Hooray, my first checkin! Hopefully I did everything right...)
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Yep, though its also useful to set the milestone to the next release version when you close the bug.
Target Milestone: --- → Thunderbird 3.3a4
(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
Flags: in-litmus?(ludovic)
https://litmus.mozilla.org/show_test.cgi?id=15381
Flags: in-litmus?(ludovic) → in-litmus+
You need to log in before you can comment on or make changes to this bug.