Investigate using `navigateTo` test helper for all navigations, even without a toolbox
Categories
(DevTools :: Framework, task, P3)
Tracking
(Not tracked)
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
.
Updated•5 years ago
|
Reporter | ||
Comment 1•3 years ago
|
||
Bug 1727506 made navigateTo work without a toolbox.
Description
•