1262880 - Remove add-on compatibility check from application update

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robert Strong (they/them - no direct email)]

According to telemetry at most 0.5% of background update checks have incompatible add-ons.
The add-ons manager code is always called when there is an app version change for an update.
There have been add-ons that override the add-ons manager code and break application update even though both the add-ons manager and application update have been coded to prevent this in most circumstances.
There have been add-ons that are essentially abandoned which will require the user to opt-in to getting an application update instead of updating silently. For example, this happened with google toolbar years ago.
Supporting the add-on compatibility check requires UI and code that is complex and it is the one piece that will prevent us from never showing UI to update if we choose to require application updates.

Comment 1

[Robert Strong (they/them - no direct email)]

Since the add-on compatibility check requires a user to opt-in to the update when an add-on is incompatible with the new version it is another step in the update process where the user might not update and hence not get security fixes as well as increase the number of users on older versions (e.g. update orphaning).

Comment 2

[Robert Strong (they/them - no direct email)]

Stephen, would you have a problem with me landing this on oak?

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: patch test changes rev1

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: patch toolkit changes rev1

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch browser changes rev1

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: patch toolkit changes rev1

Comment 7

[Robert Strong (they/them - no direct email)]

Try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=44aea05a9bdf

Comment 8

[Stephen A Pohl [:spohl]]

Comment on attachment 8745808 [details] [diff] [review]
patch toolkit changes rev1

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

not a review, just noticed this while looking this patch over:

::: toolkit/mozapps/update/content/updates.js
@@ +362,5 @@
>     * showUpdateError      nsIUpdate obj failed        either      complete  errors
>     * checkForUpdates      null          --            foreground  --        checking
>     * checkForUpdates      null          downloading   foreground  --        downloading
>     *
> +   * Note: the page returned (e.g. Result) for showUpdateAvaulable either

nit: s/showUpdateAvaulable/showUpdateAvailable/

Comment 9

[Stephen A Pohl [:spohl]]

Even though this removes a lot of code, it seems very self-contained. I'm ok if you'd like to land this on Oak. Fyi, SoftVision is actively testing bug 394984 on Oak.

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: patch toolkit changes rev2

Thanks for the drive by!

Comment 11

[Robert Strong (they/them - no direct email)]

I'll get a browser peer to review the browser bits if you aren't a browser peer yet.

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 8745807 [details] [diff] [review]
patch browser changes rev1

Forgot about the pref changes. Will resubmit this patch.

Comment 13

[Robert Strong (they/them - no direct email)]

Comment on attachment 8745808 [details] [diff] [review]
patch toolkit changes rev1

Actually I'll submit a separate patch for the pref changes.

Comment 14

[Robert Strong (they/them - no direct email)]

Also, pushed to oak. Looks like there are problems with Mac xpcshell tests due to the work being done on oak.

Comment 15

[Robert Strong (they/them - no direct email)]

Attachment: patch preference changes rev1

Pushed to oak
https://hg.mozilla.org/projects/oak/rev/85c1a69876b82ce49d16518504309f2cbea04ce1
https://hg.mozilla.org/projects/oak/rev/d75f5aa5b68fb3848353d7775e2cdc490deb00e1

https://treeherder.mozilla.org/#/jobs?repo=oak&revision=85c1a69876b8

Comment 16

[Stephen A Pohl [:spohl]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #14)
> Also, pushed to oak. Looks like there are problems with Mac xpcshell tests
> due to the work being done on oak.

Thanks for the heads up! I missed this due to all the intermittents.

Comment 17

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8745834 [details] [diff] [review]
patch toolkit changes rev2

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

For the record, this change was run by the firefox-dev mailing list [https://mail.mozilla.org/pipermail/firefox-dev/2016-April/004162.html] and did not draw any objections.

Comment 18

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8745804 [details] [diff] [review]
patch test changes rev1

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

::: toolkit/mozapps/update/tests/chrome/utils.js
@@ +156,5 @@
>  
>  // Set to true to log additional information for debugging. To log additional
>  // information for an individual test set DEBUG_AUS_TEST to true in the test's
>  // onload function.
> +var DEBUG_AUS_TEST = true;

It looks like this should get flipped back to false.

Comment 19

[Robert Strong (they/them - no direct email)]

Attachment: patch test changes rev2

Flipped DEBUG_AUS_TEST back to false

Comment 20

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8746117 [details] [diff] [review]
patch test changes rev2

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

Thanks!

Comment 21

[Robert Strong (they/them - no direct email)]

Pushed review comment to oak
https://hg.mozilla.org/projects/oak/rev/eeadb801357f

Comment 22

[Robert Strong (they/them - no direct email)]

Filed bug 1268176 for SeaMonkey
Filed bug 1268154 for Thunderbird

Comment 23

[Robert Strong (they/them - no direct email)]

Attachment: patch preference changes rev2

Just noticed that app.update.mode is sync'd so I removed that as well.

Comment 24

[Robert Strong (they/them - no direct email)]

Pushed to oak
https://hg.mozilla.org/projects/oak/rev/f753b28389da

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c66fda86cb1d
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f10077564b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/e37fe91ac862
https://hg.mozilla.org/integration/mozilla-inbound/rev/21d44aabf4a7

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c66fda86cb1d
https://hg.mozilla.org/mozilla-central/rev/3f10077564b2
https://hg.mozilla.org/mozilla-central/rev/e37fe91ac862
https://hg.mozilla.org/mozilla-central/rev/21d44aabf4a7