Closed Bug 1698158 Opened 1 month ago Closed 1 month ago

Add Services to the globals provided to the WebExtensions API modules global

Categories

(WebExtensions :: General, task, P1)

task

Tracking

(firefox88 fixed)

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: rpl, Assigned: rpl)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

Services.jsm is already loaded by the time we do load the WebExtensions API modules.
We do already import it in ExtensionCommon.jsm here, before we do create the global where the WebExtensions API modules are being executed.

As mentioned in Bug 1697404 comment 1 there are many extensions API modules (both the one integrated natively in Firefox and the experimental API part of some privileged extensions builtin into Firefox).

Given how common it is and the fact that the jsm is actually already loaded, we could provide Services by default in that global and as a side-effect reduce the chances that a privileged extension which embeds multiple experimental API could break itself by importing it in different conflicting ways (e.g. See Bug 1697404 comments 4).

Assignee: nobody → lgreco
Status: NEW → ASSIGNED

Pushed to try here (don't include talos, android and services tps test harness, but let's see how it goes with the webextensions tests and the experimental APIs likely used by all tests harnesses, e.g. quitter may be one of them):

https://treeherder.mozilla.org/jobs?repo=try&revision=d5b811b5f455db0503b926442daa2112294ea261

See Also: → 1697404

There is one issue that isn't yet addressed by these patches yet:

eslint doesn't know which globals are already defined in a webextensions API script global and so it is going to complain and the developer working on the API will likely assume that eslint is right and just import it explicitly again.

For the natively integrated APIs we can more easily list the globals that we know exist into the eslint configs in tree, but that may not help when a new builtin addon is added in tree and also when it is being developed outside of mozilla-central tree.

One strategy that we may use to improve that could be to define a plugin that does configure eslint with the list of globals exposed (we don't change that list that often and so keeping it updated shouldn't be that big of an issue), but it would still be something that the developer do need to add to their eslint config to be an effective way of avoiding that those globals would be re-imported manually.

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #8)

One strategy that we may use to improve that could be to define a plugin that does configure eslint with the list of globals exposed (we don't change that list that often and so keeping it updated shouldn't be that big of an issue), but it would still be something that the developer do need to add to their eslint config to be an effective way of avoiding that those globals would be re-imported manually.

You could set up an environment as we do for similar items here:

https://searchfox.org/mozilla-central/source/tools/lint/eslint/eslint-plugin-mozilla/lib/environments

We regularly publish new updates of eslint-plugin-mozilla to npm, so other repos could import that.

Severity: -- → S2
Priority: -- → P1
Severity: S2 → N/A

(In reply to Mark Banner (:standard8) from comment #9)

You could set up an environment as we do for similar items here:

https://searchfox.org/mozilla-central/source/tools/lint/eslint/eslint-plugin-mozilla/lib/environments

We regularly publish new updates of eslint-plugin-mozilla to npm, so other repos could import that.

That is exactly what I was thinking of, thanks a lot for pointing it out where it does live.
I'll file a separate issue to take a look into that.

Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/7793d8d8a817
part 1 - Provide Services by default in webextensions api modules. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/72d6d9258332
part 2 - Remove unnecessary jsm imports in existing WebExtensions API modules. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/05ef06f4180d
part 3.1 - Remove unnecessary imports from builtin addons experimental API. r=mixedpuppy,webcompat-reviewers,denschub
https://hg.mozilla.org/integration/autoland/rev/8a98d844e248
part 3.2 - Remove unnecessary imports from testharness addons experimental API. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/f65a4978d75e
part 3.3 - Remove unnecessary imports from talos testharness addons experimental API. r=mixedpuppy,perftest-reviewers,sparky
https://hg.mozilla.org/integration/autoland/rev/dbdead49c1c8
part 3.4 - Remove unnecessary imports from sync tps testharness experimental API. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/8ea36600327c
part 4 - Add some notes about the globals available in the WebExtensions API scripts in in-tree docs. r=mixedpuppy
Blocks: 1697404
See Also: → 1699234
Blocks: 1700184
You need to log in before you can comment on or make changes to this bug.