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

Comment 1

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

Comment 2

6 years ago
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)
Attachment #816224 - Flags: review?(standard8)
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).
(Assignee)

Comment 8

5 years ago
This is the tb hotfix addon, as a patch, suitable for https://hg.mozilla.org/releases/firefox-hotfixes/file
(Assignee)

Comment 9

5 years ago
Posted file hotfix-tb-sample.xpi (obsolete) —
The hotfix it produces
(Assignee)

Comment 10

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

Comment 12

5 years ago
Updated the versions...
Attachment #8393102 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8393101 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Attachment #8432127 - Attachment is obsolete: true
(Assignee)

Comment 14

5 years ago
Posted file hotfix-tb-sample.xpi (obsolete) —
(Assignee)

Updated

5 years ago
Depends on: 1018614
Posted file hotfix-tb-sample-signed.xpi (obsolete) —
Signed hotfix, per bug 1018614.
(Assignee)

Comment 16

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

Comment 18

5 years ago
Posted file hotfix-tb-sample.xpi (obsolete) —
Updated to have 30.0a1 and 33.0
(Assignee)

Updated

5 years ago
Attachment #8432130 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8434565 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
Attachment #816223 - Attachment is obsolete: true
(Assignee)

Comment 21

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

Comment 23

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

Comment 25

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

Comment 27

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

Comment 28

5 years ago
Attachment #8443990 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8444850 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Depends on: 1063738
(Assignee)

Comment 30

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

Comment 32

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

Comment 34

4 years ago
Changed the version number to 20150329.01.
Attachment #8485186 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 1148948
(Assignee)

Comment 35

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

Comment 38

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

Comment 39

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

Comment 41

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

Comment 42

4 years ago
Updated sample of what a hotfix would look like.
Attachment #8485187 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8459326 - Flags: review?(standard8)
Attachment #8459326 - Flags: review?(standard8) → review+
(Assignee)

Comment 43

4 years ago
https://hg.mozilla.org/comm-central/rev/afaa7fc3aab3 -> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
(Assignee)

Comment 44

4 years ago
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?

Comment 45

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

Comment 46

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

Comment 51

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