1268197 - new system add-on for "webcompat" fixes

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Robert Helmer [:rhelmer]]

We'd like to create an empty stub in `browser/extensions/webcompat` for shipping webcompat fixes after a release has gone out.

This should have a normal manifest and a `bootstrap.js` with empty startup/shutdown/install/uninstall functions (so we don't throw any errors when the add-on is started).

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

Review commit: https://reviewboard.mozilla.org/r/49341/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49341/

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

Andrew, mind taking a look?

I will find a Firefox peer to review this as well.

Comment 3

[Andrew Swan [:aswan]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

It all looks legit to me but my experience with non-webextension addons is rather limited.  Kris (:kmag) or Luca (:rpl) might have more insight.

Comment 4

[Robert Helmer [:rhelmer]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49341/diff/1-2/

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

https://reviewboard.mozilla.org/r/49341/#review46189

::: browser/extensions/webcompat/test/browser_webcompat_stub_check.js:5
(Diff revision 2)
> +"use strict";
> +
> +// make sure that the stub is present and enabled
> +add_task(function* stub_enabled() {
> +  AddonManager.getAddonByID("[e-mail]",(addon) => {

Missing a space after the comma.

Also, this task returns without waiting for the task to complete. I'd go with something like:

    let addon = yield new Promise(resolve => AddonManager.getAddonByID("[e-mail]", resolve));

::: browser/extensions/webcompat/test/browser_webcompat_stub_check.js:10
(Diff revision 2)
> +  AddonManager.getAddonByID("[e-mail]",(addon) => {
> +    Assert.notEqual(addon, null, "Webcompat stub addon should exist");
> +    Assert.equal(addon.version, "1.0");
> +    Assert.equal(addon.name, "Web Compat");
> +    Assert.ok(addon.isCompatible, "Webcompat stub addon is compatible with Firefox");
> +    Assert.ok(!addon.appDisabled, "Webcompat stub addon is not user disabled");

s/usr/app/

::: browser/extensions/webcompat/test/browser_webcompat_stub_check.js:12
(Diff revision 2)
> +    Assert.equal(addon.version, "1.0");
> +    Assert.equal(addon.name, "Web Compat");
> +    Assert.ok(addon.isCompatible, "Webcompat stub addon is compatible with Firefox");
> +    Assert.ok(!addon.appDisabled, "Webcompat stub addon is not user disabled");
> +    Assert.ok(addon.isActive, "Webcompat stub addon is active");
> +    Assert.equal(addon.type, "extension", "Webcompat stub addon is type extension");

Can we use `is`, `isnot`, and `ok` here?

Comment 6

[Robert Helmer [:rhelmer]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49341/diff/2-3/

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8746260 [details]
MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag

https://reviewboard.mozilla.org/r/49341/#review46325

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50abf674bd97

Comment 9

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: Bug 1268197 - add exception to startup IO test for webcompat system add-on r?jmaher

Review commit: https://reviewboard.mozilla.org/r/49749/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49749/

Comment 10

[Robert Helmer [:rhelmer]]

Comment on attachment 8747161 [details]
MozReview Request: Bug 1268197 - add exception to startup IO test for webcompat system add-on r?jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49749/diff/1-2/

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8747161 [details]
MozReview Request: Bug 1268197 - add exception to startup IO test for webcompat system add-on r?jmaher

https://reviewboard.mozilla.org/r/49749/#review46531

lgtm

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b498e6f7c6b0

Comment 13

[Pulsebot]

https://hg.mozilla.org/mozilla-central/rev/2b7c421063ad

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/50abf674bd97
https://hg.mozilla.org/mozilla-central/rev/2b7c421063ad

Comment 15

[Carsten Book [:Tomcat]]

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