1718279 - F5 in addon toolboxes no longer reloads the addon

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Julian Descottes [:jdescottes]]

See also https://phabricator.services.mozilla.com/D118709#inline-657795

Despite being warned by Alex about that in the review, I didn't fully retest addon reload before landing.

STRs:

• open addon toolbox from about:debugging
• go to Console panel
• evaluate this.foo = 1
• press F5
• evaluate this.foo

ER: should be undefined
AR: is still equal to 1

Note that on current Nightly, F5 breaks the console for me and I need to switch frames to restore some functionality to the toolbox. I am not sure how reliable this F5 workflow is for addons, but there's still some regression.

Overall I haven't been able to find a way to reload the remote webextension browser from the parent process, either via browser.browsingContext.reload or browser.webNavigation.reload.

If there's no parent process API that can work here we might have to reach into the content process again to perform the reload for webextensions. I would still suggest to keep the reloadBrowsingContext API on the descriptor actors and find a way to call the webextension target actor from the webextension descriptor actor.

Or we can keep the BrowsingContextTargetActor::reload API and fallback on it for webextensions.

Comment 1

[Julian Descottes [:jdescottes]]

We should absolutely add a browser mochitest asserting the actual addon F5 workflow if this is actually something supported.

Comment 2

[Julian Descottes [:jdescottes]]

After discussing with Luca (:rpl), the preferred solution here is to always fully reload the addon, which is currently only done via WebExtensionDescriptorActor::reload. This method is currently called from webext and from about:debugging's reload temporary extension button.

Today, using Ctrl + R or F5 in the toolbox has an unclear behavior. It reloads the webextension browser, but it doesn't properly reload the addon.

So currently we have two inconsistent reload behaviors for addon targets. According to Luca, Ctrl + R used to also reload the addon completely, so it's possible that the current inconsistency was introduced when creating webextension descriptors.

I will handle this in this bug, and will also add a browser mochitest. However I think it means the API name reloadBrowsingContext is not really a good fit. We could rename this to a reloadContext, reloadDescriptorContext, ... ?

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1718279 - [devtools] Addon toolbox reload should use the addon manager

Depends on D118473

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1718279 - [devtools] Rename Descriptor::reloadBrowsingContext to reloadDescriptor

Depends on D118851

The initial name no longer represents accurately what happens for webextension descriptors

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1718236

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce595859dc1b
[devtools] Addon toolbox reload should use the addon manager r=ochameau
https://hg.mozilla.org/integration/autoland/rev/f4b6178dcaed
[devtools] Rename Descriptor::reloadBrowsingContext to reloadDescriptor r=ochameau,devtools-backward-compat-reviewers

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ce595859dc1b
https://hg.mozilla.org/mozilla-central/rev/f4b6178dcaed