1267870 - about:debugging "Reload" button does not update display of metadata (name and description) in about:debugging or about:addons

Status: VERIFIED FIXED

(DevTools :: about:debugging, defect, P1)

Description

[Will Bamberg [:wbamberg]]

STR:

(1) open about:debugging
(2) click "Load Temporary Add-on"
(3) select an add-on
(4) in an editor, change the add-on's name in manifest.json
(5) click "Reload"

Expected: see the new name in about:debugging and about:addons
Actual: the name is not updated

Comment 1

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

I took a rapid look at the above STR to be able to identify what is reloaded and what is not:

- it seems that the manifest is correctly reloaded from a webextensions point of view (e.g. running "browser.runtime.getManifest()" will return the new manifest content and if we change the permission then the available chrome/browser API objects will change accordingly, e.g. add/remove "webNavigation" from the permission and realoading the addon will make chrome.webNavigation available/unavailable accordingly)

- the metadata in the about:debugging / about:addons pages is unchanged, my guess is that this metadata are cached after the initial temporary installation (which is probably the XPIDatabase[1] in the XPIProviderUtils) and never refreshed

[1]: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProviderUtils.js#425

Comment 2

[Kumar McMillan [:kumar]]

I can attempt to fix this!

Comment 3

[Kumar McMillan [:kumar]]

I have a patch in the works. The first step is bug 1269889 which will limit reloading to temporarily installed add-ons. Next step will be to flush the about:debugging cache on install.

Comment 4

[Kumar McMillan [:kumar]]

Attachment: MozReview Request: Bug 1267870 - Refresh about:debugging when add-on manifest is reloaded. r=ochameau

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

Comment 5

[Kumar McMillan [:kumar]]

Note that the about:addons bug was fixed in 1269889. My patch here addresses a bug specific to about:debugging.

Comment 6

[Kumar McMillan [:kumar]]

Comment on attachment 8751813 [details]
MozReview Request: Bug 1267870 - Refresh about:debugging when add-on manifest is reloaded. r=ochameau

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

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 8751813 [details]
MozReview Request: Bug 1267870 - Refresh about:debugging when add-on manifest is reloaded. r=ochameau

https://reviewboard.mozilla.org/r/52225/#review49373

Could you please try to fix the following exception before landing?
JavaScript error: resource://gre/modules/ExtensionContent.jsm, line 772: TypeError: extension is undefined
If happens on end of test, multiple times.

Also, I don't know if you reviewboard allow fine control on try runs,
but not that for about:debugging patches, you only need to run mochitest-devtools tests.
And only desktop platforms: win32, macxos64, linux64. (In most cases linux64 is enough).

::: devtools/client/aboutdebugging/test/browser_addons_reload.js:150
(Diff revision 2)
> +  reloadButton.click();
> +
> +  yield onAddonReloaded;
> +  // Make sure the name was updated correctly.
> +  const allReloadedNames = [...document.querySelectorAll(
> +                                              "#addons .target-name")];

nit: indentation looks weird here.
I think about:debugging tends to just wrap to a single indendation instead. So something like this:
const allReloadedNames = [...document.querySelectorAll(
  "#addons .target-name")];

::: devtools/client/aboutdebugging/test/browser_addons_reload.js:151
(Diff revision 2)
> +
> +  yield onAddonReloaded;
> +  // Make sure the name was updated correctly.
> +  const allReloadedNames = [...document.querySelectorAll(
> +                                              "#addons .target-name")];
> +  info(`reloaded names: ${allReloadedNames}`);

That prints:
reloaded names: [object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement],[object HTMLDivElement]
Which isn't really helpful.

::: devtools/client/aboutdebugging/test/browser_addons_reload.js:154
(Diff revision 2)
> +  const allReloadedNames = [...document.querySelectorAll(
> +                                              "#addons .target-name")];
> +  info(`reloaded names: ${allReloadedNames}`);
> +  const reloadedName = allReloadedNames.filter(
> +                          element => element.textContent === newName)[0];
> +  is(reloadedName && reloadedName.textContent, newName);

Looks like you are already checking textContent == newName in the filter. I think you could rename reloadedName and make it a boolean by using some() instead of filter().

Comment 8

[Kumar McMillan [:kumar]]

https://reviewboard.mozilla.org/r/52225/#review49373

I missed that. Good catch, thanks! It obvsiously doesn't cause a test failure here but it helped me track down an issue with the timing of extension shutdowns: https://bugzilla.mozilla.org/show_bug.cgi?id=1272758 It is unrelated to this patch but I'm going to work on it next.

Comment 9

[Kumar McMillan [:kumar]]

Comment on attachment 8751813 [details]
MozReview Request: Bug 1267870 - Refresh about:debugging when add-on manifest is reloaded. r=ochameau

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

Comment 10

[Kumar McMillan [:kumar]]

passing tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b1a3fea578a

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/de80d1838a8d

Comment 12

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

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

Comment 13

[Vasilica Tamas, QA [:vtamas]]

Verified fixed on Firefox 49 (20160912134115) and Firefox 51.0a1 (2016-09-14) under Windows 10 64-bit.
The name and description are successfully updated after reloading the webextension.