Closed Bug 1268197 Opened 4 years ago Closed 4 years ago

new system add-on for "webcompat" fixes

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

Details

Attachments

(2 files)

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 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.
Attachment #8746260 - Flags: feedback?(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.
Attachment #8746260 - Flags: feedback?(aswan) → feedback+
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/
Attachment #8746260 - Attachment description: MozReview Request: Bug 1268197 - stub for webcompat fix system add-on → MozReview Request: Bug 1268197 - stub for webcompat fix system add-on r?kmag
Attachment #8746260 - Flags: feedback+ → review?(kmaglione+bmo)
Attachment #8746260 - Flags: review?(kmaglione+bmo)
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("webcompat@mozilla.org",(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("webcompat@mozilla.org", resolve));

::: browser/extensions/webcompat/test/browser_webcompat_stub_check.js:10
(Diff revision 2)
> +  AddonManager.getAddonByID("webcompat@mozilla.org",(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 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/
Attachment #8746260 - Flags: review?(kmaglione+bmo)
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
Attachment #8746260 - Flags: review?(kmaglione+bmo) → review+
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/
Attachment #8747161 - Attachment description: MozReview Request: Bug 1268197 - add exception to startup IO test for webcompat system add-on → MozReview Request: Bug 1268197 - add exception to startup IO test for webcompat system add-on r?jmaher
Attachment #8747161 - Flags: review?(jmaher)
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
Attachment #8747161 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/50abf674bd97
https://hg.mozilla.org/mozilla-central/rev/2b7c421063ad
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.