956990 - Add temporary solution to checkCompatiblity=false and mass theme breakage following Australis

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Justin Dolske [:Dolske]]

After Australis landed, we saw quite a few reports of things being broken which turned out to be due to using a 3rd party theme. Themes do not get compatible-by-default treatment like add-ons do, because themes are almost certain to encounter breakage with every Firefox release. Sometimes it may be minor breakage, but any change (and especially a big change like Australis) can expose problems.

There is a pref to override the theme compatibility checking, which is presumably how some folks are having such problems (or they're manually hacking the theme, which we can't really do anything about). This pref is a footgun and should be removed.

[Alternatively, we could keep the pref as long as the theme has a minVersion of Australis -- but users would still hit such problems after Australis, so let's fix it properly.]

Comment 1

[:Gijs (he/him)]

I only see a general extensions.strictCompatibility pref, which is false by default. Which pref is this about?

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(Keeping the needinfo to keep this on my radar for when my brain isn't plague-addled.)

(In reply to :Gijs Kruitbosch from comment #1)
> I only see a general extensions.strictCompatibility pref, which is false by
> default. Which pref is this about?

This is caused by the extensions.checkCompatibility.* pref, which completely disables compatibility checking. The * is "nightly" on Nightly, or the app version string on non-Nightly.

Now that we have compatible-by-default, and we know it's working remarkably well (woo!), we can re-consider how the checkCompatibility pref works (or if it even should work). Sadly, I suspect many Nightly testers still have that pref set, and so aren't getting the benefits of compatible-by-default. But I think completely removing support for it should be seen as a last resort. It's not just Firefox affected here, although we could make it a per-app compile-time option.

My current thinking is to change the pref to be less of a giant hammer, and instead be a lot more specific to what it affects. This will solve the wider problem of the checkCompatibility pref, rather just just issues around Australis. 

ie, it would not only apply to a specific application version, but also a specific add-on. So you'd need something like:

  [e-mail] = false


There's also the question of whether themes should even be able to ignore compatibility. There's almost guaranteed to be some breakage of a theme with every Firefox update. But, we can't gauge how severe that breakage will be - very rarely will it be as severe as the Australis breakage. Sometimes it will be very minor, in which case disabling a theme will be far *more* disruptive to people. 

But, the checkCompatibility is still a very permanent thing on Nightly - I really want to make that a last resort for people. We don't do compatible-by-default for themes because we can predict with a high degree of certainty that there will be some form of breakage. But we can add an opt-in to enable compatible-by-default for themes. This would let us get some of the associated safely benefits too, such as being able to still deny compatibility to themes that weren't compatible pre-Australis. So we'd have something like:

  extensions.strictCompatibility.theme = false
  extensions.minCompatibleAppVersion.theme = 29.0


Longer term, to fix the compatibility issue with themes, I'd like to introduce a new type of theme add-on that acts as a modification to the default theme (rather than replacing it outright). This should also help bridge the gap with what people would like to do with the current lightweight theme system, but can't (and keep lightweight themes as simple as possible - possibly even allowing us to remove some options). We can't deprecate heavyweight themes at the platform level, but we can potentially phase them out at the Firefox level (already happening at the UI level with bug 520124, if I can finish it).

Comment 3

[Justin Dolske [:Dolske]]

That sounds like a much bigger fix than what's needed here. Can we just do a simple 1-time hack to ignore the pref for theme previous-to-Australis?

Comment 4

[Justin Dolske [:Dolske]]

Blair, can you pick this up and try to at least get a simple hack in before we (potentially) uplift? I Think we need to get this fixed before it hits users who are on the Aurora channel.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Yea, already on it :)

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Filed bug 961999 for changing how extensions.checkCompatibility works, and bug 962001 for allowing an opt-in for compatible-by-default for themes.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Purposefully picked a throw-away pref, so that anyone that does reset it is less likely to miss out on compat-by-default for themes when we get bug 962001. Also, the pref name should help facilitate embarrassment-driven-development.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 8362897 [details] [diff] [review]
Patch v1

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

So for non-nightly the problem is people who have set extensions.checkCompatibility.29a. Is that really anyone at this point? How many users this pref for every update?

For nightly users landing this pref won't have any impact until the next version bump, to make it take immediate effect you need to force re-calculating add-on's compatibility.

For those reasons this doesn't seem like the right fix to me.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Dave Townsend (:Mossop) from comment #8)
> So for non-nightly the problem is people who have set
> extensions.checkCompatibility.29a. Is that really anyone at this point? How
> many users this pref for every update?

It should be *much* smaller, yes. Which is why I'm not rushing like mad to get bug 961999 done before the merge. But I don't think it's non-zero.

> 
> For nightly users landing this pref won't have any impact until the next
> version bump, to make it take immediate effect you need to force
> re-calculating add-on's compatibility.

Er, yea, forgot to increment the DB schema version - which does that. Assume that patch includes that.

> For those reasons this doesn't seem like the right fix to me.

I think the only alternative is to wait until we have both bug 961999 and bug 962001. But I agree with Dolske that we're better off with a quicker fix to help reduce the pain we're experiencing right now - because Australis has shown that how we've been handling themes isn't cutting it.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8362897 [details] [diff] [review]
Patch v1

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

Fine. Do we have telemetry on compat overrides and if not can you file a bug to add it?

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/d6980b1b6751

And filed bug 965742 to add some telemetry.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d6980b1b6751

Comment 13

[Tony Mechelynck [:tonymec]]

- Does this apply to all Toolkit apps or only to those using Australis? (AFAIK, SeaMonkey doesn't.)
- Users of extensions.checkCompatibility.nightly = false ought to do it at their own risk, and make sure that they manually disable any misbehaving add-on. Whether they do so in practice is of course a different question, but even so, and even though Compatible-By-Default is really great stuff, I'm not in favour of removing the option, especially since:
  - by default it is a hidden option, its default value is Undefined, which acts the same way as true;
  - on every branch other than trunk it has to be set again with a different option name every six weeks, when the release train reaches the next station.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Tony Mechelynck [:tonymec] from comment #13)
> - Does this apply to all Toolkit apps or only to those using Australis?

Only Firefox. Although other apps can opt-in, if they really want. I doubt they want to at this stage - better to wait until bug 961999 and bug 962001, unless they do something world-breaking like Australis before then.


> - Users of extensions.checkCompatibility.nightly = false ought to do it at
> their own risk, and make sure that they manually disable any misbehaving
> add-on. Whether they do so in practice is of course a different question,
> but even so, and even though Compatible-By-Default is really great stuff,
> I'm not in favour of removing the option

Yep, agreed - which is why this bug is only a temporary measure. But checkCompatibility=false is far too big of a hammer. Bug 961999 and bug 962001 are the proper long-term solutions - it'll let people use that one stubborn add-on without the cost of manually having to deal with all their other add-ons they'd rather have automatically managed.