1075153 - Support action: disable an addon

Status: RESOLVED INCOMPLETE

(Firefox Health Report Graveyard :: Client: Desktop, defect, P4)

Description

[Benjamin Smedberg]

Implement a support action "disable addon X" by giving the addon ID.

Comment 1

[:Irving Reid (No longer working on Firefox)]

Attachment: Refactor Promise use in Self Support code

Demonstrate some ways of refactoring Promise / Task code to hopefully make it more maintainable.

Comment 2

[:Irving Reid (No longer working on Firefox)]

Attachment: Implement Self Support 'disable add-on' action

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8509529 [details] [diff] [review]
Implement Self Support 'disable add-on' action

Review of attachment 8509529 [details] [diff] [review]:
-----------------------------------------------------------------

Cancelling for now due to the below comments.
I wonder if we shouldn't just move the boiler-plate for the task-based setup into a shared JS file here.
We do this in the tests in bug 1024677 too and will probably do it again for future additions.

::: browser/components/selfsupport/SelfSupportService.js
@@ +59,5 @@
>      // throw out of the WebIDL API.
>      return this._window.Promise.resolve(() => reporter.collectAndObtainJSONPayload(true));
>    },
> +
> +  disableAddon(addonID) {

Why use this form and not |disableAddon: function(addonID) {|?

@@ +60,5 @@
>      return this._window.Promise.resolve(() => reporter.collectAndObtainJSONPayload(true));
>    },
> +
> +  disableAddon(addonID) {
> +    dump("disableAddon: " + addonID + "\n");

Left-over dump here and below.
If needed, we should add a logger.

::: browser/components/selfsupport/test/test_AddonSupport.html
@@ +41,5 @@
> +    // Disabling an unknown add-on throws
> +    let threw = false;
> +    try {
> +      yield MozSelfSupport.disableAddon("unknown@test");
> +    } catch (e) {

Let's test the error details too (presuming moving to bug ).

@@ +73,5 @@
> +      yield t();
> +    }
> +
> +    // Detach mock AddonProvider
> +    AddonManagerPrivate.unregisterProvider(MockAddonProvider);

Failures in the above tests means we don't do cleanup.
Use SimpleTest.registerCleanupFunction()?

::: dom/webidl/MozSelfSupport.webidl
@@ +47,5 @@
> +   *         Resolves with True if the browser must be restarted to disable
> +   *         the add-on, False if the add-on has been disabled without needing
> +   *         to restart.
> +   *         Rejects with an Error holding the message 'Add-on not found: <addonID>'
> +   *         if there is no add-on with the given addonID installed.

Can we get the error based on bug 1087388?

Comment 4

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #3)
> I wonder if we shouldn't just move the boiler-plate for the task-based setup
> into a shared JS file here.
> We do this in the tests in bug 1024677 too and will probably do it again for
> future additions.

Would be nice if we just had bug 1078657 instead, but i can't tell how soon that is happening.

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8509519 [details] [diff] [review]
Refactor Promise use in Self Support code

Review of attachment 8509519 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.

::: services/healthreport/healthreporter.jsm
@@ +841,5 @@
>     * @return Promise<Object | string>
>     */
> +  collectAndObtainJSONPayload(asObject=false) {
> +    return Task.spawn(function* collectJSONPayload() {
> +      dump ("collectAndObtain: " + this.toSource() + "\n");

Left-over dump here.

Comment 6

[Georg Fritzsche [:gfritzsche]]

I think this has no owner right now.

Comment 7

[Benjamin Smedberg]

When mconnor and gps wrote this up, all the support actions had a single API where the actual action was specified using a simple JSON/POD. I am not tied to that proposal, but it would make unifying this with the blocklist much easier. That's why this was marked as depending on bug 1031493.

Comment 8

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8509519 [details] [diff] [review]
Refactor Promise use in Self Support code

Should have a discussion per comment 7.

Comment 9

[Firefox Bug Husbandry Bot]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1497137; component has been deprecated.