Using `setPage()` to load a page that references the original JS file causes a loop
Categories
(WebExtensions :: Developer Tools, defect, P3)
Tracking
(firefox117 affected, firefox118 affected, firefox119 affected)
People
(Reporter: bramus, Unassigned)
Details
Attachments
(1 file)
|
118.03 KB,
image/png
|
Details |
2023-09-11_09h19_24.png
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/116.0.0.0 Safari/537.36
Steps to reproduce:
Demo REPO: https://github.com/bramus/devtools-extension-bug-setpage
I am building a DevTools extension that creates a SidebarPane for the Elements panel. I have the manifest point to a file named devtools.html as the entrypoint:
<!-- devtools.html -->
<html>
<head></head>
<body>
<script src="devtools.js"></script>
</body>
</html>
The referenced JS file contains the logic to create a SidebarPane. It also tells the sidebarPane to use an HTML page as its contents:
// devtools.js
chrome.devtools.panels.elements.createSidebarPane('POC', (sidebarPane) => {
sidebarPane.setPage('devtools.html');
});
Actual results:
The pane itself is correctly created and appears in the list of panes.
However, upon focusing the pane you get a loop: the content of devtools.html gets loaded in the pane, which loads devtools.js again, which creates new SidebarPane, which creates a new SidebarPane, which …
Expected results:
To not have a loop. This is the case in Chrome. I think they prevent files loaded in a pane via setPage() from having access to chrome.devtools.
|
||
Comment 1•2 years ago
|
||
Hello,
I reproduced the issue on the latest Nightly (119.0a1/20230910212034), Beta (118.0b7/20230910175934) and Release (117.0/20230824132758) under Windows 10 x64 and Ubuntu 22.04 LTS using the linked POC extension.
After loading the extensions via about:debugging I opened a page in a new tab (Wikipedia) and opened devtools. From the “Inspector” tab I selected an element and then from the sidebar I selected POC. As soon as the POC sidebar pane is focused, the loop occurs, creating a multitude of POC panes.
See the attached screenshot for more details.
|
|
||
Comment 2•2 years ago
|
||
|
||
Comment 3•2 years ago
|
||
(In reply to Bramus from comment #0)
Expected results:
To not have a loop. This is the case in Chrome. I think they prevent files loaded in a pane via
setPage()from having access tochrome.devtools.
I gave an explicit look to what happens in Chrome while trying to reproduce this bug using the attached test extension, and what described in comment 0 doesn't seem to be exactly how Chrome doesn't get to trigger the loop:
- The page loaded via setPage DO have access to the
chrome.devtoolsAPI (which was actually what I was expecting, otherwise the extension page set as a panel or sidebar page wouldn't be able to do much useful in the context on a devtools panel without any access to the devtools API, it could be still access it by messaging with the devtools page but that would have made writing a panel much more annoying because every extension developer would have had to bridge the API through messaging on their own) - The page loaded via setPage DO have access to the
chrome.devtools.panels.elements.createSidebarPaneAPI, and the callback is definitely being called (I made sure of that by calling it again from a Chrome DevTools toolbox opened from the sidebar panel and including a console.log to confirm the callback was being called right away), but:- if createSidebarPane is called again for the same existing sidebar panel name, then the call seems to be practically ignored (e.g. I tried to set it also to a different non -existing url and it didn't refresh the panel to load the new different url, both from the sidebar page itself and from the hidden devtools page context too, same result in both cases)
- if the reference to the parameter got from the first createSidebarPane call for a given panel name is stored by the callback on a global binding, and then I tried to call setPage again using that reference from the devtools console, then I verified that the first instance got by the devtools page is able to change the panel again (e.g. calling sidebarPage.setPage to another page url does load the new url in the panel right away), but the one got by the sidebar panel doesn't (exactly like the second one got from a devtools page if createSidebarPane is called again, which means that the sidebar panel does likely not enter in a load loop for that reason, because the instance it got for the sidebar is actually one that is ignored in practice).
We should be doing something similar in our implementation (through changes to createSidebarPane and/or setPage API methods) to avoid to hit this issue under this kind of corner case.
I'll assign it S4 as severity, given that the extensions can definitely avoid this corner case on their side (and given that this is not a recent regression, likely the behavior presented since the addition of the API, and the API affected isn't the most common API used, which restrict even more the number of extensions that can hit this in practice).
With Scroll-Driven Animations being worked on again (see https://bugzilla.mozilla.org/show_bug.cgi?id=1817303), I want to bring my Scroll-Driven Animations Debugger Extension to Firefox. This extension is affected by this bug, so I currently cannot do so.
I'll assign it S4 as severity, given that the extensions can definitely avoid this corner case on their side.
I bet you are thinking of splitting up the js code into two files:
devtools.jsto create the sidebar pane that loads the specifieddevtools.htmlfile.devtools-content.jsto be used as the script that is used indevtools.html.
While that would work in many cases, in my extension specifically I have a function that renders the UI in the DOM of the HTML file when the panel gets activated. This code to achieve this crosses the boundary of those two files:
- The “listen for activation” code should go in
devtools.js - The
renderUI()code lives indevtools-content.js
I think one could work around this by passing messages from the devtools.js file to the devtools.html file using some sort of messaging system, but I have not figured out which one that would be. And even if one were found, then it’d be cumbersome to keep variables shared by both spaces in sync.
I also tried working around this by setting window.sidebarPaneReady = true in the devtools.html file and then taking that into account before doing the call to chrome.devtools.panels.elements.createSidebarPane(…) but to no avail.
@lgreco: Do you know if there is a workaround I can use? If not, could the priority/severity of this issue be re-evaluated?
|
|
||
Comment 5•1 year ago
|
||
(In reply to Bramus from comment #4)
...
@lgreco: Do you know if there is a workaround I can use? If not, could the priority/severity of this issue be re-evaluated?
I took a quick look and the smallest change I could think of so far to workaround this issue on your extension side is to tweak the devtools.js script without splitting it out as in the following diff, change the manifest.json to point to a separate devtools-page.html (which would be loading the same devtools.js script as the devtools panel):
diff -ur ../unpacked-vanilla/devtools.js ./devtools.js
--- ../unpacked-vanilla/devtools.js 2024-05-02 18:27:20.000000000 +0200
+++ ./devtools.js 2025-02-05 18:54:12.976441993 +0100
@@ -666,7 +666,7 @@
}
};
-chrome.devtools.panels.elements.createSidebarPane('Scroll-Driven Animations', (sidebarPane) => {
+const initSidebarPanel = (sidebarPane) => {
sidebarPane.setPage('devtools.html');
sidebarPane.onShown.addListener((window) => {
activePane = window;
@@ -757,7 +757,16 @@
initPage(true);
}
});
-});
+};
+
+// Only call this method when the script is running from the hidden devtools page registered from the manifest.json.
+if (window.location.pathname === "/devtools-page.html") {
+ chrome.devtools.panels.elements.createSidebarPane('Scroll-Driven Animations', initSidebarPanel);
+} else {
+ // When running this script in the devtools sidebar page, set activePage to the current window
+ // (in the hidden devtools page that is going to be done by the initSidebarPanel callback).
+ activePane = window;
+}
chrome.runtime.onMessage.addListener(function (request, sender, sendResponse) {
if (sender.tab.id != chrome.devtools.inspectedWindow.tabId) {
Only in .: devtools-page.html
diff -ur ../unpacked-vanilla/manifest.json ./manifest.json
--- ../unpacked-vanilla/manifest.json 2024-05-02 18:27:20.000000000 +0200
+++ ./manifest.json 2025-02-05 18:49:49.662269878 +0100
@@ -14,7 +14,7 @@
"256": "images/icon-256.png",
"512": "images/icon-512.png"
},
- "devtools_page": "devtools.html",
+ "devtools_page": "devtools-page.html",
"content_scripts": [
{
"matches": ["*://*/*"],
and then added a minimal devtools-page.html like the following (given that the devtools page is invisible, it doesn't need any of the css or dom elements that are actually needed only by the sidebar panel):
<html>
<head>
<meta charset="utf-8">
<script src="devtools.js" type="module"></script>
</head>
</html>
This version didn't hit the issue tracked by bug anymore, and it seems to work as expected on Chrome (based on reproducing what the demo screencast shows).
On Firefox there may be some more work to be making it to work, at the moment it does not recognize the element as one with animations, and so it shows a more minimal UI, a node selector button and a message stating "No animations were found for the current element. Pick another element from the page", but that seems unrelated to the original issue and so this workaround may be enough to let you dig into the separate "No animatins were found..." issue hit on Firefox.
Let me know if you didn't manage to pin point what may be triggering this issue and may need some help to pin point it
(currenlty I didn't had time to dig more into the scroll-driver animations support recently landed on Nightly and compare it with what the extensions is doing to pin point if the issue is hit due to other parts still missing or because of the subtle differences between Chrome and Firefox content scripts isolation mechanisms, the following doc page on MDN provides an overview of some of the differences that extensions developers may hit, which may be useful to you too to double-check if the newissue hit after applying the workaround described above may be due to that: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts).
Thanks for your input lgreco! Your (very detailed) instructions helped me to work around this origin issue, thanks!
(Next hurdle for me to dig into is window inside the devtools.panels.ExtensionSidebarPane.onShown listener always remaining undefined – but that is food for a different issue …)
(FYI: I have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1946904 for that new hurdle)
Description
•