Skip to content

Commit

Permalink
Use gecko_android.min_version if present to make compatibility checks…
Browse files Browse the repository at this point in the history 
… against Firefox for Android (#5043)

* Use gecko_android.min_version if present to make compatibility checks against Firefox for Android
  • Loading branch information
@diox
diox authored Oct 3, 2023
1 parent 6fe2963 commit 27540bd
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 27 deletions.
60 changes: 44 additions & 16 deletions src/parsers/manifestjson.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import log from 'logger';
import * as messages from 'messages';
import JSONParser from 'parsers/json';
import {
androidStrictMinVersion,
basicCompatVersionComparison,
firefoxStrictMinVersion,
firstStableVersion,
Expand Down Expand Up @@ -143,33 +144,39 @@ export default class ManifestJSONParser extends JSONParser {
}
}

checkKeySupport(support, minVersion, key, isPermission = false) {
if (support.firefox) {
checkKeySupport(
support,
minFirefoxVersion,
minAndroidVersion,
key,
isPermission = false
) {
if (support.firefox && minFirefoxVersion) {
// We don't have to support gaps in the `@mdn/browser-compat-data`
// information for Firefox Desktop so far.
const versionAdded = support.firefox.version_added;
if (basicCompatVersionComparison(versionAdded, minVersion)) {
if (basicCompatVersionComparison(versionAdded, minFirefoxVersion)) {
if (!isPermission) {
this.collector.addWarning(
messages.keyFirefoxUnsupportedByMinVersion(
key,
this.parsedJSON.applications.gecko.strict_min_version,
minFirefoxVersion,
versionAdded
)
);
} else {
this.collector.addNotice(
messages.permissionFirefoxUnsupportedByMinVersion(
key,
this.parsedJSON.applications.gecko.strict_min_version,
minFirefoxVersion,
versionAdded
)
);
}
}
}

if (support.firefox_android) {
if (support.firefox_android && minAndroidVersion) {
// `@mdn/browser-compat-data` sometimes provides data with gaps, e.g., a
// feature was supported in Fennec (added in 56 and removed in 79) and
// then re-added in Fenix (added in 85) and this is expressed with an
Expand All @@ -182,20 +189,22 @@ export default class ManifestJSONParser extends JSONParser {
// not warn if the minVersion is in one of the few version gaps).
const versionAddedAndroid = firstStableVersion(support.firefox_android);

if (basicCompatVersionComparison(versionAddedAndroid, minVersion)) {
if (
basicCompatVersionComparison(versionAddedAndroid, minAndroidVersion)
) {
if (!isPermission) {
this.collector.addWarning(
messages.keyFirefoxAndroidUnsupportedByMinVersion(
key,
this.parsedJSON.applications.gecko.strict_min_version,
minAndroidVersion,
versionAddedAndroid
)
);
} else {
this.collector.addNotice(
messages.permissionFirefoxAndroidUnsupportedByMinVersion(
key,
this.parsedJSON.applications.gecko.strict_min_version,
minAndroidVersion,
versionAddedAndroid
)
);
Expand All @@ -204,19 +213,31 @@ export default class ManifestJSONParser extends JSONParser {
}
}

checkCompatInfo(compatInfo, minVersion, key, manifestKeyValue) {
checkCompatInfo(
compatInfo,
minFirefoxVersion,
minAndroidVersion,
key,
manifestKeyValue
) {
for (const subkey in compatInfo) {
if (Object.prototype.hasOwnProperty.call(compatInfo, subkey)) {
const subkeyInfo = compatInfo[subkey];
if (subkey === '__compat') {
this.checkKeySupport(subkeyInfo.support, minVersion, key);
this.checkKeySupport(
subkeyInfo.support,
minFirefoxVersion,
minAndroidVersion,
key
);
} else if (
typeof manifestKeyValue === 'object' &&
Object.prototype.hasOwnProperty.call(manifestKeyValue, subkey)
) {
this.checkCompatInfo(
subkeyInfo,
minVersion,
minFirefoxVersion,
minAndroidVersion,
`${key}.${subkey}`,
manifestKeyValue[subkey]
);
Expand All @@ -226,7 +247,8 @@ export default class ManifestJSONParser extends JSONParser {
) {
this.checkKeySupport(
subkeyInfo.__compat.support,
minVersion,
minFirefoxVersion,
minAndroidVersion,
`${key}:${subkey}`,
true
);
Expand Down Expand Up @@ -638,14 +660,20 @@ export default class ManifestJSONParser extends JSONParser {
}
}

const minVersion = firefoxStrictMinVersion(this.parsedJSON);
if (!this.isLanguagePack && !this.isDictionary && minVersion) {
const minFirefoxVersion = firefoxStrictMinVersion(this.parsedJSON);
const minAndroidVersion = androidStrictMinVersion(this.parsedJSON);
if (
!this.isLanguagePack &&
!this.isDictionary &&
(minFirefoxVersion || minAndroidVersion)
) {
for (const key in bcd.webextensions.manifest) {
if (Object.prototype.hasOwnProperty.call(this.parsedJSON, key)) {
const compatInfo = bcd.webextensions.manifest[key];
this.checkCompatInfo(
compatInfo,
minVersion,
minFirefoxVersion,
minAndroidVersion,
key,
this.parsedJSON[key]
);
Expand Down
17 changes: 17 additions & 0 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,23 @@ export function firefoxStrictMinVersion(manifestJson) {
return null;
}

export function androidStrictMinVersion(manifestJson) {
if (
manifestJson.applications &&
manifestJson.applications.gecko_android &&
manifestJson.applications.gecko_android.strict_min_version &&
typeof manifestJson.applications.gecko_android.strict_min_version ===
'string'
) {
return parseInt(
manifestJson.applications.gecko_android.strict_min_version.split('.')[0],
10
);
}
// Fall back on gecko.min_version if gecko_android.min_version isn't provided
return firefoxStrictMinVersion(manifestJson);
}

export function basicCompatVersionComparison(versionAdded, minVersion) {
const asNumber = parseInt(versionAdded, 10);
return !Number.isNaN(asNumber) && asNumber > minVersion;
Expand Down
97 changes: 86 additions & 11 deletions tests/unit/parsers/test.manifestjson.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,64 @@ describe('ManifestJSONParser', () => {
expect(manifestJSONParser.isValid).toEqual(true);
});

it('should warn when gecko.strict_min_version is set below 113.0 and gecko_android is present', () => {
const addonLinter = new Linter({ _: ['bar'] });
const json = validManifestJSON({
browser_specific_settings: {
gecko: {
strict_min_version: '100.0',
},
gecko_android: {},
},
});

const manifestJSONParser = new ManifestJSONParser(
json,
addonLinter.collector
);

const { warnings } = addonLinter.collector;
expect(warnings).toEqual([
expect.objectContaining({
code: messages.KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION,
description: oneLine`"strict_min_version" requires Firefox for
Android 100, which was released before version 113 introduced
support for "browser_specific_settings.gecko_android".`,
}),
]);
expect(manifestJSONParser.isValid).toEqual(true);
});

it('should warn when gecko_android.strict_min_version is set below 113.0', () => {
const addonLinter = new Linter({ _: ['bar'] });
const json = validManifestJSON({
browser_specific_settings: {
gecko: {
strict_min_version: '113.0',
},
gecko_android: {
strict_min_version: '100.0',
},
},
});

const manifestJSONParser = new ManifestJSONParser(
json,
addonLinter.collector
);

const { warnings } = addonLinter.collector;
expect(warnings).toEqual([
expect.objectContaining({
code: messages.KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION,
description: oneLine`"strict_min_version" requires Firefox for
Android 100, which was released before version 113 introduced
support for "browser_specific_settings.gecko_android".`,
}),
]);
expect(manifestJSONParser.isValid).toEqual(true);
});

describe('browser_style', () => {
function parseManifest(manifestVersion, manifestKey, browserStyleValue) {
const manifest = {
Expand Down Expand Up @@ -4316,12 +4374,17 @@ describe('ManifestJSONParser', () => {
);
});

const _checkKeySupport = ({ support, minVersion }) => {
const _checkKeySupport = ({
support,
minFirefoxVersion,
minAndroidVersion,
}) => {
const isPermission = true;

manifestJSONParser.checkKeySupport(
support,
minVersion,
minFirefoxVersion,
minAndroidVersion,
key,
isPermission
);
Expand All @@ -4335,13 +4398,17 @@ describe('ManifestJSONParser', () => {
],
};

_checkKeySupport({ support, minVersion: 40 });
_checkKeySupport({
support,
minFirefoxVersion: 42,
minAndroidVersion: 54,
});

expect(addonLinter.collector.notices).toHaveLength(1);
expect(addonLinter.collector.notices[0]).toEqual(
expect.objectContaining({
description: oneLine`"strict_min_version" requires Firefox for
Android 48.0.0, which was released before version 56 introduced
Android 54, which was released before version 56 introduced
support for "${key}".`,
})
);
Expand All @@ -4352,21 +4419,25 @@ describe('ManifestJSONParser', () => {
firefox_android: [
{ version_added: true },
{ version_added: false },
{ version_added: '56', version_removed: '79' },
{ version_added: '72', version_removed: '79' },
{ version_added: NaN },
{ version_added: '23', version_removed: '25' },
{ version_added: '52', version_removed: '56' },
{ version_added: '85' },
{ version_added: null },
],
};

_checkKeySupport({ support, minVersion: 10 });
_checkKeySupport({
support,
minFirefoxVersion: 42,
minAndroidVersion: 50,
});

expect(addonLinter.collector.notices).toHaveLength(1);
expect(addonLinter.collector.notices[0]).toEqual(
expect.objectContaining({
description: oneLine`"strict_min_version" requires Firefox for
Android 48.0.0, which was released before version 23 introduced
Android 50, which was released before version 52 introduced
support for "${key}".`,
})
);
Expand All @@ -4375,17 +4446,21 @@ describe('ManifestJSONParser', () => {
it('supports an object compat data for Android', () => {
const support = {
firefox_android: {
version_added: '85',
version_added: '114',
},
};

_checkKeySupport({ support, minVersion: 10 });
_checkKeySupport({
support,
minFirefoxVersion: 42,
minAndroidVersion: 113,
});

expect(addonLinter.collector.notices).toHaveLength(1);
expect(addonLinter.collector.notices[0]).toEqual(
expect.objectContaining({
description: oneLine`"strict_min_version" requires Firefox for
Android 48.0.0, which was released before version 85 introduced
Android 113, which was released before version 114 introduced
support for "${key}".`,
})
);
Expand Down

0 comments on commit 27540bd

Please sign in to comment.