From 66eca9e88cf24531d0a5cd4e4b1c2f3de8f3afdf Mon Sep 17 00:00:00 2001
From: Julian Descottes <jdescottes@mozilla.com>
Date: Mon, 13 Feb 2023 17:52:51 +0000
Subject: [PATCH] Bug 1757457 - [devtools] Recompute mCanExecuteScripts after
 navigation r=smaug

I can see that after navigation, allowJavascript is correctly set for the browsing context, but it seems we need to call
RecomputeCanExecuteScripts() to force it to be applied to the new page. Not sure if doing this here makes sense or if it should be done earlier.

Also there are still other issues with this feature:
- closing the toolbox does not reload the page: meaning JavaScript remains disabled on the page
- similarly all pages which have been put in bfcache will retain the javascript disabled/enabled setting

For the first issue, I wonder if we should force a reload when closing the toolbox (iff javascript disabled was toggled).
And for the second issue, could we invalidate contexts put in bfcache for a given browsing context when we toggle allowJavaScript?

Olli: Does this change make sense at least to fix the basic issue?

Differential Revision: https://phabricator.services.mozilla.com/D169182
---
 .../browser_toolbox_options_disable_js.js     |  7 ++++++
 docshell/base/BrowsingContext.cpp             |  2 ++
 docshell/test/unit/test_allowJavascript.js    | 25 +++++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/devtools/client/framework/test/browser_toolbox_options_disable_js.js b/devtools/client/framework/test/browser_toolbox_options_disable_js.js
index 0018420e791e..20458a7eb592 100644
--- a/devtools/client/framework/test/browser_toolbox_options_disable_js.js
+++ b/devtools/client/framework/test/browser_toolbox_options_disable_js.js
@@ -21,6 +21,13 @@ add_task(async function() {
   await testJSDisabled();
   await testJSDisabledIframe();
 
+  // Navigate and check JS is still disabled
+  for (let i = 0; i < 10; i++) {
+    await navigateTo(`${TEST_URI}?nocache=${i}`);
+    await testJSDisabled();
+    await testJSDisabledIframe();
+  }
+
   // Re-enable JS.
   await toggleJS(toolbox);
 
diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp
index e5e8720e9071..b20b52bf776d 100644
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -610,6 +610,8 @@ void BrowsingContext::SetDocShell(nsIDocShell* aDocShell) {
   if (mChildSessionHistory) {
     mChildSessionHistory->SetIsInProcess(true);
   }
+
+  RecomputeCanExecuteScripts();
 }
 
 // This class implements a callback that will return the remote window proxy for
diff --git a/docshell/test/unit/test_allowJavascript.js b/docshell/test/unit/test_allowJavascript.js
index e9d2f60fca3b..cab251f465b8 100644
--- a/docshell/test/unit/test_allowJavascript.js
+++ b/docshell/test/unit/test_allowJavascript.js
@@ -261,8 +261,29 @@ add_task(async function() {
   bc.reload(0);
   await AllowJavascriptParent.promiseLoad(bc);
 
-  await assertLoadFired(bc, undefined, "top BC with scripts disabled");
-  await assertScriptsAllowed(bc, false, "top BC with scripts disabled");
+  await assertLoadFired(
+    bc,
+    undefined,
+    "top BC with scripts disabled after reload"
+  );
+  await assertScriptsAllowed(
+    bc,
+    false,
+    "top BC with scripts disabled after reload"
+  );
+
+  await page.loadURL("http://example.org/?other");
+
+  await assertLoadFired(
+    bc,
+    undefined,
+    "top BC with scripts disabled after navigation"
+  );
+  await assertScriptsAllowed(
+    bc,
+    false,
+    "top BC with scripts disabled after navigation"
+  );
 
   await page.close();
   Services.prefs.clearUserPref("dom.security.https_first");