If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Panel content scripts don't work after reloading or changing location

RESOLVED FIXED

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: wbamberg, Assigned: ochameau)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
See: http://groups.google.com/group/mozilla-labs-jetpack/browse_thread/thread/275dc226dbb033bc/0bfe727b87be4f65?lnk=gst&q=err_destroyed#0bfe727b87be4f65. 

If a panel displays content from a URL and the location is changed or the page is reloaded, then that panel's content scripts stop working, raising the error:

ERR_DESTROYED, "The page has been destroyed and can no longer be used."

As the group suggests, using an iframe is an effective workaround, but we should consider whether this should be fixed.
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → Future
Whiteboard: [triage:followup]
On second thought, this sounds pretty bad, especially if it happens when a page is merely reloaded (document.location changes may not be as common, but pages get reloaded by users quite regularly).  Retriaging.
Assignee: nobody → poirot.alex
Priority: P2 → P1
Whiteboard: [triage:followup]
Target Milestone: Future → 1.1
I hit this the other day. (This actually is even worse than the location changing case, since my example doesn't even change locations!)

In my add-on, I had a panel with three links that can be clicked. When they are clicked, the panel's contentScript sends an event back to the main add-on script for it to do something. I set the links' href attribute to |""| so that they don't actually go anywhere (but they get the default link styling because the href attribute is present).
But because of this bug, clicking the links kills the contentScript completely, so that the custom event never gets emitted.

I got around it by removing the empty href attribute and just giving the links a link-esque styling via CSS.
(Assignee)

Comment 3

6 years ago
Created attachment 551789 [details] [diff] [review]
Handle panel document location change

This patch allows to handle panel location changes by executing the content script on each document loaded into the panel.

Wes: You can avoid <a> links to change location by using <a href="#">link</a>.
Attachment #551789 - Flags: review?(myk)
Comment on attachment 551789 [details] [diff] [review]
Handle panel document location change

This looks good, I don't see anything wrong with it, but I'm seeing a variety of test failures on Mac after applying it.  The failures are inconsistent but are mostly various tests in test-selection.  And they seem to be associated with the added test, since the failures go away when I remove the new test from test-panel.js, even if I don't actually remove the implementation change.
Attachment #551789 - Flags: review?(myk) → review-
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: 1.1 → ---
(Assignee)

Comment 6

6 years ago
Created attachment 565266 [details] [diff] [review]
Remove duplicate call to waitUntilDone

That's not the first time I got into such issue: I let duplicate calls to test.waitUntilDone(). That always leads to weird issue in some test after the related one ...
Attachment #551789 - Attachment is obsolete: true
Attachment #565266 - Flags: review?(myk)
Comment on attachment 565266 [details] [diff] [review]
Remove duplicate call to waitUntilDone

Aha!

How risky do you think this patch is?
Attachment #565266 - Flags: review?(myk) → review+
Whiteboard: [cherry-pick-wanted]
(Assignee)

Comment 8

6 years ago
I know it is a common and painfull issue, but this fix may uncover edge cases with:
 - scripts that already contain bugfix around this,
 - content script been executed multiple times that end up breaking stuff.
Ok, then let's land it in the development branch but not cherry-pick it to stabilization.
Whiteboard: [cherry-pick-wanted]
(Assignee)

Comment 10

6 years ago
Landed in master:
https://github.com/mozilla/addon-sdk/commit/9591dd292243fb05ab890e7f1a0ea22d59dbed2c
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.