Closed Bug 1616207 Opened 5 years ago Closed 3 years ago

Investigate using `navigateTo` test helper for all navigations, even without a toolbox

Categories

(DevTools :: Framework, task, P3)

task

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1727506

People

(Reporter: ochameau, Unassigned)

References

Details

During bug 1615283's review, where I introduced navigateTo test helper, Nicolas raised a good point about the fact that this method only works with a toolbox already opened. And it may be useful to use it in any case of navigation, no matter if we have a toolbox or not:
https://phabricator.services.mozilla.com/D62805#inline-378491
https://phabricator.services.mozilla.com/D62804#inline-379352

We could possibly do it somewhat like that:

diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js
index 96ded8e87b15..6ed7db38fea6 100644
--- a/devtools/client/shared/test/shared-head.js
+++ b/devtools/client/shared/test/shared-head.js
@@ -266,6 +266,22 @@ var refreshTab = async function(tab = gBrowser.selectedTab) {
 async function navigateTo(uri, { isErrorPage = false } = {}) {
   const target = await TargetFactory.forTab(gBrowser.selectedTab);
   const toolbox = gDevTools.getToolbox(target);
+  if (!toolbox) {
+    info(`Load document "${uri}" without a toolbox`);
+    const browser = gBrowser.selectedBrowser;
+    const onBrowserLoaded = BrowserTestUtils.browserLoaded(
+      browser,
+      false,
+      null,
+      isErrorPage
+    );
+    await BrowserTestUtils.loadURI(browser, uri);
+
+    info(`Waiting for page to be loaded…`);
+    await onBrowserLoaded;
+    info(`→ page loaded`);
+    return;
+  }
   const currentToolboxTarget = toolbox.target;
 
   // If fission and target switching are enabled, and if we're switching origin, we need

My issue with this is that, if, for some unexpected reason, gDevTools.getToolbox(target) returns null, we won't wait for any toolbox event. So that we may not wait for any event, if this helper code fails fetching the toolbox.

But it may be unecessary fear. The navigateTo helper landed and was applied to many tests and none of them seem to be throwing because of a null toolbox.

So we may proceed with that if no particular intermittent surfaces.
If we do that, in theory, we should no longer use BrowserTestUtils.loadURI and there will be opportunities to introduce an eslint rule to forbid it in favor of navigateTo.

Priority: -- → P3

Bug 1727506 made navigateTo work without a toolbox.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.