Support hotfix add-on in Thunderbird

RESOLVED FIXED in Thunderbird 40.0

Status

defect
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Irving, Assigned: mkmelin)

Tracking

unspecified
Thunderbird 40.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird38 fixed, thunderbird39 fixed, thunderbird40 fixed)

Details

Attachments

(4 attachments, 13 obsolete attachments)

3.45 KB, patch
standard8
: review+
Details | Diff | Splinter Review
36.82 KB, application/x-xpinstall
Details
17.16 KB, application/x-xpinstall
Details
24.23 KB, patch
Details | Diff | Splinter Review
Thunderbird does not currently support the hotfix add-on mechanism Firefox has (bug 694068); this limits our ability to respond to situations such as https://bugzilla.mozilla.org/show_bug.cgi?id=913918#c8
Posted patch bug914225_tb-hotfix.patch (obsolete) — Splinter Review
Turns out this is very easy to do. Just adding suitable prefs.

Makes the necessary ports from
 o Bug 694068 - Automatically download and install an available hotfix add-on
 o Bug 711679 - Send background version checks to new domain
   => makes hotfixes come from extensions.update.background.url
 o Bug 704988 - Check the hotfix add-on is signed by a specific certificate
   + Bug 707207 - Get an object signing certificate (or two!) for signing the hotfix add-ons
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #816223 - Flags: review?(mbanner)
Attachment #816223 - Flags: feedback?(dtownsend+bugmail)
Posted file thunderbird-hotfix-test-files.zip (obsolete) —
Toolkit's got this all tested. To make sure it works in thunderbird too I tested it manually with this. (See the README for what to do)
Comment on attachment 816223 [details] [diff] [review]
bug914225_tb-hotfix.patch

Review of attachment 816223 [details] [diff] [review]:
-----------------------------------------------------------------

You're going to have to be careful to update your cert at the same rate as Firefox
Attachment #816223 - Flags: feedback?(dtownsend+bugmail) → feedback+
Comment on attachment 816223 [details] [diff] [review]
bug914225_tb-hotfix.patch

On its own, this patch looks fine. Though I'd like to get feedback from Ben or one of the other guys that we are OK to use the same signing mechanisms for the Thunderbird add-on.

(I'm happy for us to use the same cert etc).

I also need to review the add-on itself still.
Attachment #816223 - Flags: review?(standard8)
Attachment #816223 - Flags: review+
Attachment #816223 - Flags: feedback?(bhearsum)
Comment on attachment 816223 [details] [diff] [review]
bug914225_tb-hotfix.patch

Review of attachment 816223 [details] [diff] [review]:
-----------------------------------------------------------------

As far as I know this should be fine.
Attachment #816223 - Flags: feedback?(bhearsum) → feedback+
Comment on attachment 816224 [details]
thunderbird-hotfix-test-files.zip

This looks great.

There's a couple of typos on this line in the readme file:

 - make the thunerbird-hotfix.xpi file available on http://localhost:8080/thunderbirdb-hotfix.xpi 

The icon*.png files should be replaced by the Thunderbird equivalents (should be able to get these from other-licenses).
Attachment #816224 - Flags: review?(standard8) → review+
So I guess the next steps are:

1) Get releng to sign the xpi with their cert
2) Upload it to AMO (copy FF pages, and get review etc)
3) Test a Thunderbird with the main patch and verify it works
4) Land the patch

Magnus, can you drive these forward?

I have some accounts I use for Thunderbird related add-ons, so we could use one of those as primary author for the add-on (and I'm happy to set the page up to upload it, once we've got the signed version).
This is the tb hotfix addon, as a patch, suitable for https://hg.mozilla.org/releases/firefox-hotfixes/file
Posted file hotfix-tb-sample.xpi (obsolete) —
The hotfix it produces
(In reply to Mark Banner (:standard8) from comment #7)
> So I guess the next steps are:
> 
> 1) Get releng to sign the xpi with their cert

Do you know who to contact for this? The add-on is now attached to this bug as attachment 8393102 [details].
Flags: needinfo?(standard8)
(In reply to Magnus Melin from comment #10)
> (In reply to Mark Banner (:standard8) from comment #7)
> > So I guess the next steps are:
> > 
> > 1) Get releng to sign the xpi with their cert
> 
> Do you know who to contact for this? The add-on is now attached to this bug
> as attachment 8393102 [details].

I've just realised the best way to do this would be to file a bug in "Release Engineering" and get them to do it there.
Flags: needinfo?(standard8)
Updated the versions...
Attachment #8393102 - Attachment is obsolete: true
Attachment #8393101 - Attachment is obsolete: true
Attachment #8432127 - Attachment is obsolete: true
Posted file hotfix-tb-sample.xpi (obsolete) —
Depends on: 1018614
Posted file hotfix-tb-sample-signed.xpi (obsolete) —
Signed hotfix, per bug 1018614.
(In reply to Mark Banner (:standard8) from comment #7)
> So I guess the next steps are:
> 
> 1) Get releng to sign the xpi with their cert
> 2) Upload it to AMO (copy FF pages, and get review etc)
> 3) Test a Thunderbird with the main patch and verify it works
> 4) Land the patch
> 
> Magnus, can you drive these forward?
> 
> I have some accounts I use for Thunderbird related add-ons, so we could use
> one of those as primary author for the add-on (and I'm happy to set the page
> up to upload it, once we've got the signed version).

So, time for step 2. The signed xpi is attachment 8434565 [details].
Flags: needinfo?(standard8)
Bah:

+        <em:minVersion>30.0a</em:minVersion>

isn't a valid version number in AMO, needs to be 30.a1 (https://addons.mozilla.org/en-US/thunderbird/pages/appversions/).

So this fails the validation :-(
Flags: needinfo?(standard8)
Posted file hotfix-tb-sample.xpi (obsolete) —
Updated to have 30.0a1 and 33.0
Attachment #8432130 - Attachment is obsolete: true
Attachment #8434565 - Attachment is obsolete: true
Attachment #816223 - Attachment is obsolete: true
(In reply to Nick Thomas [:nthomas] from comment #20)
> Created attachment 8444850 [details]
> hotfix-tb-sample-signed.xpi
> 
> Signed copy of attachment 8443988 [details].

Mark, can you get this uploaded to amo.
Flags: needinfo?(standard8)
This is now on AMO, currently awaiting review:

https://addons.mozilla.org/en-US/thunderbird/addon/thunderbird-update-hotfix/
Flags: needinfo?(standard8)
The hotfix add-on is now reviewed. 
I applied the patch, and things work with one caveat. The version should not be in the format I used ("tb-sample"), so to test it you you need to set the pref extensions.hotfix.lastVersion to something that makes the version check update us, e.g. "sb-sample" yes, with starting s. Versions should (of course) be of the format described in nsIVersionComparator.idl. I'm not sure what changed since I originally wrote the patch though. Anyway, if we ever need a hotfix we can use a correct version and things will work.

I'd suggest we now
 1) remove the add-on from amo
 2) land the patch

Let me know when the add-on is removed from amo.
Flags: needinfo?(standard8)
We can't delete it from AMO - that would run the risk of someone else re-using the id. I'm not sure if disabling it would adversely affect things later.

If we land the patch, but keep it enabled, it would auto-apply & remove from existing users, but I don't see a big issue in that.

Should we fix the version number issue before pushing the patch live though? Is it going to not work if we push one with a correct version number later?
Flags: needinfo?(standard8)
A correct version later will work. 
But, I propose that we fix the version number and set the min and max version numbers to say 33.0a2. That would make it testable for the (few) users running such a build, but not installed for any other builds. How does that sound? 

Since it's to remain on amo, the description verbiage should be fixed to say "... enables Thunderbird to provide" (not Firefox) - https://addons.mozilla.org/en-US/thunderbird/addon/thunderbird-update-hotfix/
(In reply to Magnus Melin from comment #25)
> A correct version later will work. 
> But, I propose that we fix the version number and set the min and max
> version numbers to say 33.0a2. That would make it testable for the (few)
> users running such a build, but not installed for any other builds. How does
> that sound? 

Sounds good.

> Since it's to remain on amo, the description verbiage should be fixed to say
> "... enables Thunderbird to provide" (not Firefox) -
> https://addons.mozilla.org/en-US/thunderbird/addon/thunderbird-update-hotfix/

Oops, thanks. Fixed.
Updated version that accepts 33.0a1 only
Attachment #816224 - Attachment is obsolete: true
Attachment #8432129 - Attachment is obsolete: true
Attachment #8443988 - Attachment is obsolete: true
Attachment #8443990 - Attachment is obsolete: true
Attachment #8444850 - Attachment is obsolete: true
Depends on: 1063738
Mark: please upload the new version to amo for review
Flags: needinfo?(standard8)
Bah, unfortunately I can't upload the new version as the version number "tb-sample" is the same.

It looks like I could disable it, so that might have the same effect.
Flags: needinfo?(standard8)
So do we need a new version with changed version number? And do you still want to have it under that account, or should I create a new one and handle the amo stuff myself?
Flags: needinfo?(standard8)
(In reply to Magnus Melin from comment #32)
> So do we need a new version with changed version number?

Yes.

> And do you still
> want to have it under that account, or should I create a new one and handle
> the amo stuff myself?

I'm quite happy to transition that account to someone else, or just add more accounts onto the add-on (they can have more than one owner).
Flags: needinfo?(standard8)
Changed the version number to 20150329.01.
Attachment #8485186 - Attachment is obsolete: true
Depends on: 1148948
The signed 20150329.01 version is now available in attachment 8585592 [details]. 
Let me know regarding transition of the account, or adding my account to the add-on.
Flags: needinfo?(standard8)
Ok, I've uploaded it and also added your email address as an author for the add-on.
Flags: needinfo?(standard8)
with comment 36 and bug 1148948 done, what's left to do?
Flags: needinfo?(mkmelin+mozilla)
So I just tested the live version. Unfortunately I somehow managed to mess up the last extension version so it does not actually do anything, as bootstrap.js is in the wrong place. Doh!! 

It does get "installed" though.

To test it
0) ensure app.update.enabled and app.update.auto are true
1) Get a build from here: http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2014/06/2014-06-19-03-02-05-comm-central/
2) set the prefs that are set by the patch in attachment 8459326 [details] [diff] [review]
3) Wait until background check occurs, or execute this from the Error Console:
   Components.utils.import("resource://gre/modules/AddonManager.jsm");AddonManagerPrivate.startup();AddonManagerPrivate.backgroundUpdateCheck();

In a few seconds, the add-on will get installed.


In the (bash) console I saw 
1430767877474	addons.xpi	WARN	Error loading bootstrap.js for thunderbird-hotfix@mozilla.org: Error opening input stream (invalid filename?)

---

As there wasn't really any functionality to test, I suppose that it got installed (and showed in the add-on manager) is enough to prove that it works. I think the patch is ready to land. 

Standard8, are you ok with landing it now?
Flags: needinfo?(mkmelin+mozilla) → needinfo?(standard8)
If not, we can of course do another signing round.
I think as long as we don't end up with the dead version installed in the profiles when we enable the hotfix, then that's fine.

I suspect that might be we just need to check the version limitations on it are correct? but probably worth validating first.
Flags: needinfo?(standard8)
(In reply to Mark Banner (:standard8) from comment #40)
> I think as long as we don't end up with the dead version installed in the
> profiles when we enable the hotfix, then that's fine.
> 
> I suspect that might be we just need to check the version limitations on it
> are correct? but probably worth validating first.

Yes it's set to install for 33.0a1 only. I just retested with trunk and that didn't get hotfix installed (as expected).
The add-on would only end up installed if you run with 33.0a1, which I expect is pretty much nobody except us testing.
Updated sample of what a hotfix would look like.
Attachment #8485187 - Attachment is obsolete: true
Attachment #8459326 - Flags: review?(standard8)
Attachment #8459326 - Flags: review?(standard8) → review+
https://hg.mozilla.org/comm-central/rev/afaa7fc3aab3 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
Comment on attachment 8459326 [details] [diff] [review]
proposed fix (unbitrotted)

[Approval Request Comment]

Pref changes only. We should take this after some trunk baking.
Attachment #8459326 - Flags: approval-comm-beta?
Attachment #8459326 - Flags: approval-comm-aurora?
Did this break mozmill? Is there some trivial typo or a bug that causes Tb to fail early in many tests?
The failures aren't there before this checkin. Or is it m-c again?
Does it work locally? Won't be able to check until tomorrow.
More likely perhaps, the same mozharness changes that caused the current xpcshell bustage.
Comment on attachment 8459326 [details] [diff] [review]
proposed fix (unbitrotted)

https://hg.mozilla.org/releases/comm-aurora/rev/6e77ddf85e80
https://hg.mozilla.org/releases/comm-beta/rev/1bf43580b6f2
Attachment #8459326 - Flags: approval-comm-beta?
Attachment #8459326 - Flags: approval-comm-beta+
Attachment #8459326 - Flags: approval-comm-aurora?
Attachment #8459326 - Flags: approval-comm-aurora+
What ever happened to this addon? The link https://addons.mozilla.org/en-US/thunderbird/addon/thunderbird-update-hotfix/ says the addon is disabled/removed.
We should make sure we have credentials for the account this has been uploaded with and/or more unlisted owners. Maybe one of you knows what is going on.
Flags: needinfo?(standard8)
Flags: needinfo?(mkmelin+mozilla)
(In reply to Philipp Kewisch [:Fallen] from comment #49)
> We should make sure we have credentials for the account this has been
> uploaded with and/or more unlisted owners. Maybe one of you knows what is
> going on.

It looks like Magnus has currently disabled the add-on. The credentials shouldn't be shared, all the permissions necessary can be set by adding more owners. Tb-drivers should decide who they are.
Flags: needinfo?(standard8)
Yes, Mark added me to that add-on account. The add-on is now disabled since it's not something that people should install, just a proof the hotfix functionality works.
Flags: needinfo?(mkmelin+mozilla)
You need to log in before you can comment on or make changes to this bug.