1447831 - Remove front-end support for non-restartless add-ons

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Kris Maglione [:kmag]]

This includes about:addons and door hanger install flow.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions.

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

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1447831: Part 2 - Remove support for restart-required install prompts.

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

Comment 3

[Code Review Bot [:reviewbot]]

Comment on attachment 8961198 [details]
Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions.

https://reviewboard.mozilla.org/r/229972/#review235666


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/mozapps/extensions/test/browser/browser_updateid.js:11
(Diff revision 1)
>  
>  var gProvider;
>  var gManagerWindow;
>  var gCategoryUtilities;
>  
>  var gApp = document.getElementById("bundle_brand").getString("brandShortName");

Error: 'gapp' is assigned a value but never used. allowed unused vars must match /^end_test$/. [eslint: no-unused-vars]

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8961198 [details]
Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions.

https://reviewboard.mozilla.org/r/229972/#review235918

::: toolkit/mozapps/extensions/content/extensions.xml:1125
(Diff revision 1)
> -          if (pending != AddonManager.PENDING_NONE) {
> +          if (pending & AddonManager.PENDING_UNINSTALL) {
>              this.removeAttribute("notification");
>  
> -            pending = null;
> -            const PENDING_OPERATIONS = ["enable", "disable", "install",
> -                                        "uninstall", "upgrade"];
> +            // We don't care about pending operations other than uninstall.
> +            // They're transient, and cannot be undone.
> +            this.setAttribute("pending", "uninstall");
> -            for (let op of PENDING_OPERATIONS) {
> -              if (this.isPending(op))
> -                pending = op;
> -            }
> -
> -            this.setAttribute("pending", pending);
>              this._pending.textContent = gStrings.ext.formatStringFromName(
> -              "notification." + pending,
> -              [this.mAddon.name, gStrings.brandShortName], 2
> +              "notification.restartless-uninstall",
> +              [this.mAddon.name], 1);
> -            );

I think this whole clause will never be reached since we'll replace the binding:
https://searchfox.org/mozilla-central/rev/de5c4376b89c3603341d226a65acea12f8851ec5/toolkit/mozapps/extensions/content/extensions.css#34-36

Comment 5

[Andrew Swan [:aswan]]

Comment on attachment 8961198 [details]
Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions.

https://reviewboard.mozilla.org/r/229972/#review235918

> I think this whole clause will never be reached since we'll replace the binding:
> https://searchfox.org/mozilla-central/rev/de5c4376b89c3603341d226a65acea12f8851ec5/toolkit/mozapps/extensions/content/extensions.css#34-36

whoops, its not true that "the whole clause will never be reached" but we'll never display "notification.restartless-uninstall"

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8961198 [details]
Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions.

https://reviewboard.mozilla.org/r/229972/#review236210

r=me with `notification.restartless-uninstall` removed (or an explanation of why the comment about it is wrong)

::: toolkit/mozapps/extensions/test/browser/browser_details.js
(Diff revision 1)
> -      run_next_test();
> -    });
> -  });
> -});
> -
> -// Opens and tests the details view for add-on 8

also remove addon8 from where the addons are created at the top

Comment 7

[Andrew Swan [:aswan]]

Comment on attachment 8961199 [details]
Bug 1447831: Part 2 - Remove support for restart-required install prompts.

https://reviewboard.mozilla.org/r/229974/#review236214

Why am I not surprised that no tests needed to be changed or deleted for this...

Comment 8

[Kris Maglione [:kmag]]

(In reply to Andrew Swan [:aswan] from comment #7)
> Why am I not surprised that no tests needed to be changed or deleted for
> this...

That's because I already changed or deleted them in bug 1446686 :)

Comment 10

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6275a8fabb9723cbd0689cfa2897adad784ca7a9
Bug 1447831: Part 1 - Remove about:addons support for restart-required extensions. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/b854a20546fef478d8b87f0a92a881f4f83ebe8c
Bug 1447831: Part 2 - Remove support for restart-required install prompts. r=aswan

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6275a8fabb97
https://hg.mozilla.org/mozilla-central/rev/b854a20546fe

Comment 12

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.