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

RESOLVED FIXED in Thunderbird 5.0b1

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: standard8, Assigned: squib)

Tracking

Trunk
Thunderbird 5.0b1
Bug Flags:
in-litmus +

Firefox Tracking Flags

(blocking-thunderbird5.0 needed)

Details

(Whiteboard: [gs], URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
blocking-thunderbird5.0: --- → needed
Whiteboard: [gs]

Comment 1

8 years ago
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.
(Assignee)

Comment 2

8 years ago
Created attachment 514740 [details] [diff] [review]
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.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #514740 - Flags: review?(bugzilla)
(Assignee)

Comment 3

8 years ago
Actually... I guess we'd still want the addon install dialog/warning to pop up, right?
(Assignee)

Comment 4

8 years ago
Created attachment 514745 [details] [diff] [review]
Display the install addon dialog too

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)
(Assignee)

Comment 5

8 years ago
Created attachment 517227 [details] [diff] [review]
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.
Attachment #514745 - Attachment is obsolete: true
Attachment #517227 - Flags: review?(bugzilla)
Attachment #514745 - Flags: review?(bugzilla)
(Reporter)

Comment 6

8 years ago
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-
(Assignee)

Comment 7

8 years ago
Created attachment 519857 [details] [diff] [review]
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.
Attachment #517227 - Attachment is obsolete: true
Attachment #519857 - Flags: ui-review?(clarkbw)
Attachment #519857 - Flags: review?(bugzilla)
Attachment #517227 - Flags: ui-review?(clarkbw)
(Reporter)

Comment 8

8 years ago
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+
(Assignee)

Comment 10

8 years ago
Adding checkin-needed. Are there litmus tests for this already?
Keywords: checkin-needed
(Assignee)

Comment 11

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

(Hooray, my first checkin! Hopefully I did everything right...)
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Reporter)

Comment 12

8 years ago
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.