panel.contentURL change doesn't swap out the visible page in a panel



Add-on SDK
5 years ago
4 months ago


(Reporter: clarkbw, Assigned: irakli)


(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)


Changing the contentURL of a panel does change the location (your content script will be notified of a location change) however it doesn't seem to change the visible page.  While there is a test for the panel location changing a panel I don't think it actually tests the change in frame contents.

Here's a simple test case of changing contentURL on timeouts

Similar question on Stack Overflow

And here's the change I found fixed the issue by swapping out the frame loader before the _initFrame call.

diff --git a/packages/addon-kit/lib/panel.js b/packages/addon-kit/lib/panel.js
index edc5581..9832c27 100644
--- a/packages/addon-kit/lib/panel.js
+++ b/packages/addon-kit/lib/panel.js
@@ -354,6 +354,7 @@ const Panel = Symbiont.resolve({
   _onChange: function _onChange(e) {
     if ('contentURL' in e && this._frame) {
+      this._frameLoadersSwapped = false;
       // Cleanup the worker before injecting the content script in the new
       // document

Original Message Here:!topic/mozilla-labs-jetpack/a5wjP1clkCo



5 years ago
Assignee: nobody → evold
Priority: -- → P1
related bug 574868
Priority: P1 → --
Priority: -- → P1
Thanks for the patch Bryan!  So far your patch seems to work for me on your test case, but I think that there may be a better fix for the problem.  Afaict you are switching the panel frame and a hidden frame it uses, but I think this might mean that changing the contentURL is changing the url of the wrong frame.

Alex or Irakli can either of you take a look at this, it seems like a follow up to bug 574868 and I'm not very familiar enough with the panel implementation yet I think.  I could use a explanation of how the panel uses it's hidden frame.  As I recall it is supposed to use the hidden frame only when the panel is hidden, and swap it when the panel is showing.  If that is correct then the target frame for changes might be getting mixed up.
Flags: needinfo?(rFobic)
Once new document is loaded `_onShow` is called:

That would cause swapping frame loaders back, so this patch will not break anything.
More interesting question is why swapping frame loaders is required in first place. My guess would be platform bug / limitation in frame loaders that won't draw new page after location change.

That being said I think we should land Bryan's patch and also investigate why swapping loaders is actually required in a separate bug. We should also add comment to this change to mention that frame loaders have to be swapped to workaround platform limitation / bug pointing to that other bug.
Flags: needinfo?(rFobic)
Assignee: evold → rFobic
I updated the PR.
Blocks: 821476

Comment 5

5 years ago
Commits pushed to master at
fix for bug #795490 - panel.contentURL change doesn't swap out the visible page in a panel
Merge pull request #594 from canuckistani/bug_795490

fix Bug 795490 r=@gozala


5 years ago
Last Resolved: 5 years ago
Resolution: --- → FIXED
we should always have a test guys.. I don't think this was actually fixed.
Resolution: FIXED → ---
Duplicate of this bug: 859007
I'm going through the list of open bugs that github robot has commented in. Is this bug fixed, Erik?
Flags: needinfo?(evold)
(In reply to Wes Kocher (:KWierso) from comment #8)
> I'm going through the list of open bugs that github robot has commented in.
> Is this bug fixed, Erik?

Doesn't look like it.
Flags: needinfo?(evold)

Comment 10

4 months ago
This is SDK and isn't going to get fixed.
Last Resolved: 5 years ago4 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.