1754441 - Deprecate InstallTrigger

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, task, P2)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

+++ This bug was initially created as a clone of Bug #1442035 +++

Short summary of the InstallTrigger use cases

• installing an addon

  • addons.mozilla.org doesn't use InstallTrigger anymore as a falling back mechanism (to be precise it looks that we removed it from mozilla/addons-frontend#6750, around the end of 2018)

  • some websites may still be using it to install their addon from the websites itself (but it is likely that they may be as well using a link to the xpi file or navigate a window to the xpi url, which in the end are the officially supported installation methods meant to be used by non addons-mozilla.org websites)

• UA detection

  • web-vitals is not using it anymore (removed in GoogleChrome/web-vitals#124)

  • some web frameworks may still be using it (e.g. EmberJS seems to still uses it for that purpose here, we do even have a couple of copies of that in-tree coming from EmberJS examples included in a couple of tests: https://searchfox.org/mozilla-central/search?q=typeof+InstallTrigger&path=)

  • some websites may still be using it (e.g. because if they are using an older web-vitals version, or some framework that is still using it internaly, or because of their own custom UA detection code, likely derived from a stack overflow question that provides that same typeof InstallTrigger !== 'undefined' snippet in the top answer

Deprecation plan

Instead of removing it right away (which seems to be what Bug 1442035 was aiming for), we are considering to start by:

• logging an explicit deprecation warning (through the Deprecated Web IDL extended attribute)

• make sure we are going to be collect telemetry (if I'm not mistaken using this WebIDL attribute should also automatically collect telemetry for it as a side-effect) and that we can derive from it how much of its usage may be due to just the "UA detection" vs. the "install addons" use case

• remove from it old cruft (e.g. methods that are not doing anymore what they were originally intended for) and some corner use cases that were never supposed to be supported

• add an about:config pref (and/or RemoteSetting) to hide all methods but leaves the InstallTrigger objectas defined (and so fully remove the "install addons" support without overlapping with the "UA detection" issues)

• add a separate about:config pref (and/or RemoteSetting) to hide the InstallTrigger itself (in preparation to fully remove it if our telemetry makes us confident enough that the "UA detection" use case isn't going to be a problem anymore)

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Add an explicit deprecation warning on InstallTrigger.

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Explicitly check for supported web schemes. r?zombie!

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Add prefs to control InstallTrigger/InstallTriggerImpl visibility.

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Explicit set prefs to enable InstallTrigger on all tests that depend on its availability.

Depends on D138452

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Add RemoteSettings to control InstallTrigger/InterTriggerImpl visibility.

Depends on D138793

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - List extensions.InstallTrigger/InstallTriggerImpl.enabled prefs in about:support.

Depends on D138794

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Add extensions.InstallTrigger/InstallTriggerImpl.enabled prefs to telemetry environment.

Depends on D138795

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f72c3caea300
Explicitly check for supported web schemes. r=zombie

Comment 9

[Cosmin Sabou [:CosminS]]

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

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: request_bug1754441_telemetryEnvironment_InstallTrigger_prefs.md

Data review request for the two new prefs added to list of prefs collected in the telemetryEnvironment userPrefs property (related to the phabricator revision: https://phabricator.services.mozilla.com/D138796).

Comment 11

[Chris H-C :chutten]

Comment on attachment 9264831 [details]
request_bug1754441_telemetryEnvironment_InstallTrigger_prefs.md

PRELIMINARY NOTES:
An individual must be named as the one responsible for non-expiring collections, so I've volunteered you, :rpl : D

If this isn't correct, please supply a different individual who will be responsible for this collection.

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Luca Greco is responsible.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does the data collection use a third-party collection tool?

No.

---

Result: datareview+

Comment 12

[Luca Greco [:rpl] [:luca] [:lgreco]]

PRELIMINARY NOTES:
An individual must be named as the one responsible for non-expiring collections, so I've volunteered you, :rpl : D

If this isn't correct, please supply a different individual who will be responsible for this collection.

Hi Chris,
thanks for needinfo-ing me along with the data review, I shouldn't have forgot that, my apologies.

I was just chatting about this with Shane and we were wondering if there can be more than one individual (e.g. to have a fallback, "just in case").
is that allowed / possible?

In any case, I'm ok to be the individual responsible for this collection.

Comment 13

[Chris H-C :chutten]

The more the merrier! There needs to be at least one specific individual listed in the review response. (Or, in this case, below the review response). More than one is groovy.

Comment 14

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1754441 - Add in-tree docs for AMRemoteSettings. r?robwu!,leplatrem

Depends on D138796

Comment 15

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Chris H-C :chutten from comment #13)

The more the merrier! There needs to be at least one specific individual listed in the review response. (Or, in this case, below the review response). More than one is groovy.

Hi Mike,
given that you may be the one monitoring this collection more directly, do you want to be listed as individual responsible for this collection along with me?

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

Push to try for the entire stack of patches (after having pushed to phabricator one last round of updates and rebased the entire stack on top of today's mozilla-central tip):

• https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2d9720ba83a30208d7ee55cd0b2f4684c96c7f7

Comment 17

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Thomas,
as I briefly described in comment 0 InstallTrigger deprecation does also have potential webcompat side-effects, and so I wanted to make sure to provide to the webcompat team an "heads-up" and some details about it up front.

This set of patches is now all signed-off and so it should land soon enough in Firefox 100, but it is not going to hide the InstallTrigger global or its methods just yet.

On the contrary once we will land these patches, Firefox will start to emit deprecation warnings when the InstallTrigger global is accessed or its InstallTrigger.install method called (there are two separate deprecation warning messages, and so there will be also two separate deprecation counters in telemetry too).

There are some more details in planning documents, I'm about to share those with you as well, to make sure you can have a good picture of the next steps of the deprecation plans and we can discuss further about it.

Comment 18

[Karl Dubost💡 :karlcow]

There is probably a need for an intent to deprecate on dev-platform.

In terms of webcompat, installTrigger is currently used as a way to detect Firefox.

https://github.com/webcompat/web-bugs/issues/98497#issuecomment-1025404718
https://github.com/webcompat/web-bugs/issues/82928#issuecomment-933145083
https://github.com/webcompat/web-bugs/issues/89116#issuecomment-936671603
https://github.com/webcompat/web-bugs/issues/75078#issuecomment-854385928
https://github.com/webcompat/web-bugs/issues/68738#issuecomment-812339501
https://github.com/webcompat/web-bugs/issues/46313#issuecomment-565581586
https://github.com/webcompat/web-bugs/issues/21650#issuecomment-460529140

The pattern is often a variation of:

var isFirefox = typeof InstallTrigger !== "undefined";

In some cases removing installTrigger will solve the webcompat issues but in some other cases it will probably create breakages by sending Firefox in codepaths that were not planned by the web developers.

So expect breakages in nightly once this is released. And we could have big surprises.

Comment 19

[Karl Dubost💡 :karlcow]

We probably need telemetry for window.installTrigger
https://github.com/search?l=JavaScript&q=installTrigger+firefox&type=Code

Comment 20

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Karl Dubost💡 :karlcow from comment #19)

We probably need telemetry for window.installTrigger
https://github.com/search?l=JavaScript&q=installTrigger+firefox&type=Code

I definitely agree, that should actually be already covered by D138302, the Deprecated WebIDL extended attribute does have an implicit side-effect of collecting data in an auto-generated telemetry histogram, in particular:

• accessing the InstallTrigger global collects telemetry in the following two histograms:
  • USE_COUNTER2_DEPRECATED_InstallTriggerDeprecated_PAGE (on the parent process) / USE_COUNTER2_DEPRECATED_InstallTriggerDeprecated_DOCUMENT (on the content process)
• accessing the InstallTrigger.install methods collects telemetry in the following two histograms:
  • USE_COUNTER2_DEPRECATED_InstallTriggerInstallDeprecated_PAGE (on the parent process) / USE_COUNTER2_DEPRECATED_InstallTriggerInstallDeprecated_DOCUMENT (on the content process)

Comment 21

[Luca Greco [:rpl] [:luca] [:lgreco]]

(clearing Thomas needinfo, he already handled it by forwarding the request to the rest of the webcompat Team, thanks a lot Thomas!).

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74d84f0f64ac
Add an explicit deprecation warning on InstallTrigger. r=mixedpuppy,emilio
https://hg.mozilla.org/integration/autoland/rev/5e55e93b5336
Add prefs to control InstallTrigger/InstallTriggerImpl visibility. r=mixedpuppy,webidl,smaug
https://hg.mozilla.org/integration/autoland/rev/095054f6f110
Explicit set prefs to enable InstallTrigger on all tests that depend on its availability. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/ddf908d08916
Add RemoteSettings to control InstallTrigger/InterTriggerImpl visibility. r=robwu,leplatrem
https://hg.mozilla.org/integration/autoland/rev/cc415c5944ed
List extensions.InstallTrigger/InstallTriggerImpl.enabled prefs in about:support. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/4b572b213e40
Add extensions.InstallTrigger/InstallTriggerImpl.enabled prefs to telemetry environment. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/dc10a99012c0
Add in-tree docs for AMRemoteSettings. r=leplatrem,robwu

Comment 23

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/74d84f0f64ac
https://hg.mozilla.org/mozilla-central/rev/5e55e93b5336
https://hg.mozilla.org/mozilla-central/rev/095054f6f110
https://hg.mozilla.org/mozilla-central/rev/ddf908d08916
https://hg.mozilla.org/mozilla-central/rev/cc415c5944ed
https://hg.mozilla.org/mozilla-central/rev/4b572b213e40
https://hg.mozilla.org/mozilla-central/rev/dc10a99012c0

Comment 24

[Alex Cornestean]

Verified the fix on the latest Nightly (100.0a1/20220328213646) under Windows 10 x64, Ubuntu 16.04 LTS and macOS 11.3.1.

InstallTrigger and InstallTrigger.install are currently controlled by new about:config prefs - “extensions.InstallTrigger.enabled” and “extensions.InstallTriggerImpl.enabled” set by default to “true”.

Flipping “extensions.InstallTrigger.enabled” to false will hide InstallTrigger global completely and flipping “extensions.InstallTriggerImpl.enabled” to false will make InstallTrigger global to be still visible but set to null.

Corresponding deprecation warning messages are logged to console when accessing the deprecated features:

• “InstallTrigger is deprecated and will be removed in the future” when accessing InstallTrigger
• “InstallTrigger.install() is deprecated and will be removed in the future. For more help https://extensionworkshop.com/documentation/publish/self-distribution/" when accessing InstallTrigger.install

Telemetry is properly collected when accessing the deprecated features:

• Accessing the InstallTrigger global collects telemetry in the following two histograms:
  USE_COUNTER2_DEPRECATED_InstallTriggerDeprecated_PAGE
  USE_COUNTER2_DEPRECATED_InstallTriggerDeprecated_DOCUMENT
• Accessing the InstallTrigger.install methods collects telemetry in the following two histograms:
  USE_COUNTER2_DEPRECATED_InstallTriggerInstallDeprecated_PAGE
  USE_COUNTER2_DEPRECATED_InstallTriggerInstallDeprecated_DOCUMENT

The new prefs are properly shown in both about:support and about:telemetry once changed from their default values

The prefs can also be properly set/reverted via the RemoteSettings “main/addons-manager-settings” collection with the same expected results as when manually setting the prefs.

Comment 25

[Karl Dubost💡 :karlcow]

Webcompat Regression https://github.com/webcompat/web-bugs/issues/101685

Comment 26

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Karl Dubost💡 :karlcow from comment #25)

Webcompat Regression https://github.com/webcompat/web-bugs/issues/101685

I'm moving the link to the webcompat regression issue to Bug 1759737, the install page from www.dashlane.com is hitting the error raised on InstallTrigger.install called without user activation (user input requirement.

Comment 27

[Alex]

For UA detection, is there an alternative to InstallTrigger (not based on user agent)?

I know that in theory one should not use browser detection at all, but there are subtle differences in browsers and especially when using JS rendering frameworks (like React), you sometimes need to adapt component behavior specific to browsers...

Just as an example: Autofill/Autocomplete is not working at all in FF with react standard input elements unless you do some workarounds (and will probably not be fixed)
https://github.com/facebook/react/issues/18986

Comment 28

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Dennis,
I'm needinfo-ing you to highlight for the webcompat team the InstallTrigger UserAgent detection use case mentioned in comment 27.
(as a side note, https://github.com/facebook/react/issues/18986 comments are then also pointing to Bug 1642570, which in its wontfix comment points to https://github.com/facebook/react/issues/15739).

Comment 29

[Alex]

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

I'm needinfo-ing you to highlight for the webcompat team the InstallTrigger UserAgent detection use case mentioned in comment 27.
(as a side note, https://github.com/facebook/react/issues/18986 comments are then also pointing to Bug 1642570, which in its wontfix comment points to https://github.com/facebook/react/issues/15739).

There are always situations where browsers act differently and also frameworks have bugs in a specific browser, just like the bugs in React you linked. E.g. in our app there are two situations where we need to do something special just in Firefox, with no other way (like interface/feature detection) than browser detection.

So even if undesirable, there is a need for detecting whether JS is executed by Firefox (or some other browser). If your recommendation is to e.g. check against the user agent, then this is fine for me and we will use that instead of the InstallTrigger detection mechanism.
I would just like to hear your recommendation, thanks you!

Best regards,
Alex

Comment 30

[Dennis Schubert [:denschub]]

If no feature detection is possible, I agree that this is a bit of a bad spot to be in. In those cases, running the simplest user-agent based test, something like

const isFirefox = /firefox/i.test(navigator.userAgent);

is probably the best way here.

Relying on Firefox-only properties to detect the browser itself might look like an easier solution, but unfortunately it has the potential of breaking down the line - or it can force the browser developers to keep a non-standard property around just for compatibility reasons, which also isn't great.

Comment 31

[Alex]

(In reply to Dennis Schubert [:denschub] from comment #30)

Relying on Firefox-only properties to detect the browser itself might look like an easier solution, but unfortunately it has the potential of breaking down the line - or it can force the browser developers to keep a non-standard property around just for compatibility reasons, which also isn't great.

I agree, thank you!

Comment 32

[Yves Goergen]

Considering how the UA detection tests look like for other browsers, it seems like Firefox wants to be the only browser with no custom API members at all. We have navigator.brave, window.chrome, window.opera and window.safari. Maybe the best test is to see if it's any of these, and if there are none of the other browsers' identification features, it must be "stealth" Firefox. ;-)

I'd be happy to change my UA detection method (and get rid of the warning in the console) but I prefer something that cannot be spoofed by the user because the user cannot simply fix Firefox bugs by changing their UA string. Like the bug that WebSocket connections are closed immediately when a navigation starts, and not when the page actually unloads. In this case, I want to disable the WebSocket reconnect for a few seconds to avoid another short connection for the time between navigation start and page unload. And Firefox on Android has some performance issues that Chrome does not, so disabling certain effects there is also good for the user. Both are uncritical and things won't fall apart when this test fails. It's just a minor optimisation to make the world a little bit nicer (and hide some Firefox bugs from the user to let Firefox shine a little more).

So, can we please have a non-UA-string browser detection like all others have one?

Comment 33

[Simon Mainey]

Yves, try harder :) https://arkenfox.github.io/TZP/tests/engine.html and that is barely scratching the surface, e.g. -moz attributes/keys/properties