addons-linter icon indicating copy to clipboard operation
addons-linter copied to clipboard

Declaring gecko_android in a manifest breaks old versions of Firefox

Open dotproto opened this issue 2 years ago • 16 comments
trafficstars

Describe the problem and steps to reproduce

  1. Create a manifest.json file with the following contents:
    {
      "name": "Manifest test",
      "version": "1.0",
      "manifest_version": 2,
      "browser_specific_settings": {
        "gecko": {
          "id": "person@example"
          "strict_min_version": "42"
        },
        "gecko_android": {}
      }
    }
    
  2. Use addons-linter to lint this manifest.

What happened?

addons-linter does not show a minimum Firefox version compatibility warning if "browser_specific_settings.gecko_android" is declared in the manifest. This may lead to unexpected behaviors as older versions of Firefox will refuse to load an extension if this property is included in an extension's manifest.

What did you expect to happen?

The linter should log a warning and indicate that "gecko_android" is not compatible with Firefox versions below 78.

Anything else we should know?

@diox noted that MDN compatibility data for "gecko_android" was adjusted in https://github.com/mdn/browser-compat-data/pull/20810 to address false warnings that were returned by addon-linter.

This issue was originally reported by @ghostwords in a comment on the Mozilla Add-ons blog.

https://blog.mozilla.org/addons/2023/10/05/changes-to-android-extension-signing/comment-page-1/#comment-227718

Note that adding gecko_android to browser_specific_settings might mean having to raise your extension’s minimum required version of Firefox.

browser_specific_settings.gecko_android makes Firefox 60 fail to load your extension with “There was an error during installation: Extension is invalid”.

Firefox 68 fails with “There was an error during installation: Extension is invalid

Reading manifest: Error processing browser_specific_settings: Unexpected property “gecko_android”.

Firefox 78 appears to be the earliest ESR version to successfully load extensions with browser_specific_settings.gecko_android

┆Issue is synchronized with this Jira Task

dotproto avatar Oct 11 '23 23:10 dotproto

I think this is an issue with browser_specific_settings not allowing extra properties in versions < 78 or something.

Edit: so in fact, this should work in Firefox version 69 and above, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1542351

willdurand avatar Oct 12 '23 08:10 willdurand

Side note: When did we introduce browser_specific_settings in favor of applications?

wagnerand avatar Oct 12 '23 08:10 wagnerand

It was made "official" a year ago or so, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1797777 and https://bugzilla.mozilla.org/show_bug.cgi?id=1797050 but, in practice, it was much older than that.

willdurand avatar Oct 12 '23 09:10 willdurand

FYI I have received a bug report from a user of my add-on once I added gecko_android, about the add-on not working after an update. Perhaps we should update AMO to consider the presence of gecko_android as an implicit declaration of browser_specific_settings.gecko.strict_min_version = 69.

Rob--W avatar Nov 06 '23 18:11 Rob--W

By the way, I added a blank gecko_android to Privacy Badger's manifest, and now AMO claims that the minimum supported version on Android is 113. Is it really though? Regardless, this was an unwelcome surprise.

ghostwords avatar Nov 06 '23 21:11 ghostwords

By the way, I added a blank gecko_android to Privacy Badger's manifest, and now AMO claims that the minimum supported version on Android is 113. Is it really though? Regardless, this was an unwelcome surprise.

The gecko_android key was introduced in version 113. If you don't specify strict_min_version, Firefox for Android version 113 is chosen instead. If you want to explicitly support more older versions for some reason, set strict_min_version to a lower value.

Rob--W avatar Nov 07 '23 20:11 Rob--W

That is not at all intuitive. The expected behavior is that a blank "gecko_android" will inherit strict_min_version from "gecko".

ghostwords avatar Nov 07 '23 20:11 ghostwords

That is not at all intuitive. The expected behavior is that a blank "gecko_android" will inherit strict_min_version from "gecko".

Why would that be expected? gecko_android is not a child of gecko, so I would not expect it to inherit.

wagnerand avatar Nov 07 '23 21:11 wagnerand

The expectation @ghostwords mentioned is reflected in the current documentation on MDN.

strict_min_version … If not provided, defaults to the version determined by gecko.strict_min_version.

dotproto avatar Nov 07 '23 21:11 dotproto

That is not at all intuitive. The expected behavior is that a blank "gecko_android" will inherit strict_min_version from "gecko".

Why would that be expected? gecko_android is not a child of gecko, so I would not expect it to inherit.

Logically gecko specifies the behavior for Firefox. gecko_android specifies Android-specific behavior, but if not set, then it is reasonable to expect gecko to be used as a default.

In Firefox, that is already how it works: gecko_android overrides gecko.

Rob--W avatar Nov 07 '23 21:11 Rob--W

The expectation @ghostwords mentioned is reflected in the current documentation on MDN.

strict_min_version … If not provided, defaults to the version determined by gecko.strict_min_version.

Thanks! I read that as property not provided, not value not provided. But I agree it's not entirely clear.

How about we make the value mandatory? In other words, if you specify the property, you also need to provide the value (and it cannot be empty).

wagnerand avatar Nov 07 '23 21:11 wagnerand

AMO will use strict_min_version from gecko if gecko_android is present and blank. However, because gecko_android is used, the minimum version AMO will set no matter what for Android will be 113.0 in that situation.

diox avatar Nov 07 '23 22:11 diox

How about we make the value mandatory? In other words, if you specify the property, you also need to provide the value.

That does not offer any meaningful benefits. We shouldn't require too much boilerplate.

We chose to default to 113 because that was the first version where gecko_android was recognized by Firefox. 113 was released ½ year ago. It's risky for users to be using old unsupported versions of a browser. On mobile they should be using the last or recent version, due to auto updates through the Play store.

General availability of add-ons is rolling out now (near version 120). Before that, the only way to install arbitrary add-ons is through the Collection feature, which is only available on pre-release versions of Firefox for Android:

  • Firefox Nightly >= 83
  • Firefox Beta >= 107

The main Firefox app hasn't supported arbitrary extensions after 68 (which was released 3+ years ago and should not be used by anyone who cares about security).

In short, I would not expect much negative impact from setting the minimum version to 113+. On the contrary, developers don't have to be concerned about user reports from ancient unsupported Firefox for Android releases.

Rob--W avatar Nov 07 '23 22:11 Rob--W

In short, I would not expect much negative impact from setting the minimum version to 113+. On the contrary, developers don't have to be concerned about user reports from ancient unsupported Firefox for Android releases.

I hope that's true! I checked Privacy Badger's AMO stats page yesterday. I think it looked like roughly 6% of Android users were on Firefox older than 113, which is not nothing.

ghostwords avatar Nov 07 '23 22:11 ghostwords

There are two issues reported in this issue:

  1. The initial issue, about addons-linter not outputting any warnings when the extension is incompatible with Firefox 68 and older (except for ESR 68) when gecko_android is set in browser_specific_settings.
    • The general issue here is that the linter does not recognize when there has been a change in the strictness of unrecognized key enforcement. We should add a warning when this happens.
  2. @ghostwords 's comment about gecko_android forcing minimum compatibility of add-ons on Android to 113. This is unrelated to the issue here. If there is a need to take any action, it would either be a documentation issue (to file at https://github.com/mozilla/extension-workshop/) or an AMO server issue (if there is a request to support lower versions), to file at https://github.com/mozilla/addons-server/.

In short, I would not expect much negative impact from setting the minimum version to 113+. On the contrary, developers don't have to be concerned about user reports from ancient unsupported Firefox for Android releases.

I hope that's true! I checked Privacy Badger's AMO stats page yesterday. I think it looked like roughly 6% of Android users were on Firefox older than 113, which is not nothing.

Ah, Privacy Badger is one of the few extensions that were part of the default collection, available to users on release. That explains the higher-than-expected usage. Users on these ancient unsupported Firefox versions would then just be stuck on an older version of your add-on. They should update to the latest Firefox version, because continuing to use ancient browser versions will result in exposure to already-resolved security vulnerabilities, and encountering broken websites that use modern web platform features.

If you ever get user reports complaining about the inability to install the add-on, just encourage them to update their browser. As an extension developer myself, I have also received emails from users in this situation, and my recommendation for them was to update the browser.

Rob--W avatar Nov 09 '23 15:11 Rob--W

If you want to explicitly support more older versions for some reason, set strict_min_version to a lower value.

I did just that, setting strict_min_version to 78 for Android: https://github.com/EFForg/privacybadger/commit/46676e52bd3e0ae5d3c312a7513c4ab4f7411932

However, AMO's "Manage Status & Versions" page for the new version still shows a disabled compatibility dropdown for Android, still fixed to 113: https://addons.mozilla.org/en-US/developers/addon/privacy-badger17/versions/5659023

AMO screenshot

ghostwords avatar Dec 01 '23 19:12 ghostwords