From 6ab57dcec5db79101507c6490089c35da9528be9 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Thu, 8 Apr 2021 11:48:52 +0000 Subject: [PATCH] Bug 1702078 - [devtools] a11y panel should emit ready event after rendering the UI r=ochameau,perftest-reviewers With the previous implementation, the toolbox would resolve the panel before all the initialization data had been retrieved. This could lead to issues if we tried to destroy the toolbox right after the panel got selected. Differential Revision: https://phabricator.services.mozilla.com/D110247 --- ...r_aboutdebugging_devtoolstoolbox_reload.js | 20 +------------------ devtools/client/accessibility/panel.js | 10 ++++++---- ...owser_accessibility_relation_navigation.js | 11 +++++++++- .../browser/browser_accessibility_sidebar.js | 11 +++++++++- .../browser_accessibility_tree_nagivation.js | 11 +++++++++- ...owser_accessibility_tree_nagivation_oop.js | 11 +++++++++- .../accessibility/accessibility-helpers.js | 6 +----- 7 files changed, 48 insertions(+), 32 deletions(-) diff --git a/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_reload.js b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_reload.js index 9a0c2fabbe62..c15a66523517 100644 --- a/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_reload.js +++ b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_reload.js @@ -61,25 +61,12 @@ async function testReloadAboutDevToolsToolbox(toolId) { const toolbox = getToolbox(devtoolsWindow); await toolbox.selectTool(toolId); - // The a11y panel finishes its initialization after selectTool resolves. - // TODO: The a11y panel init should only resolve when the UI is ready and all - // initial requests are completed. See Bug 1702078. - if (toolId === "accessibility") { - await waitForA11yPanel(toolbox); - } - info("Wait for requests to settle before reloading"); await toolbox.target.client.waitForRequestsToSettle(); info("Reload about:devtools-toolbox page"); devtoolsBrowser.reload(); - const newToolbox = await gDevTools.once("toolbox-ready"); - - // Again, wait for the delayed a11y panel initialization. - if (toolId === "accessibility") { - await waitForA11yPanel(newToolbox); - } - + await gDevTools.once("toolbox-ready"); ok(true, "Toolbox is re-created again"); // Check that about:devtools-toolbox is still selected tab. See Bug 1570692. @@ -98,8 +85,3 @@ async function testReloadAboutDevToolsToolbox(toolId) { await closeAboutDevtoolsToolbox(document, devtoolsTab, window); await removeTab(tab); } - -async function waitForA11yPanel(toolbox) { - const panel = toolbox.getPanel("accessibility"); - await panel._accessibilityViewInitialized; -} diff --git a/devtools/client/accessibility/panel.js b/devtools/client/accessibility/panel.js index ae6c5e036f26..1e418d7605d3 100644 --- a/devtools/client/accessibility/panel.js +++ b/devtools/client/accessibility/panel.js @@ -103,10 +103,6 @@ AccessibilityPanel.prototype = { EVENTS.ACCESSIBILITY_INSPECTOR_UPDATED, this.onAccessibilityInspectorUpdated ); - // Expose a promise so that tests can wait for the UI to be ready. - this._accessibilityViewInitialized = this.panelWin.once(EVENTS.INITIALIZED); - - this.shouldRefresh = true; this.accessibilityProxy = new AccessibilityProxy(this._commands); this.accessibilityProxy.startListeningForTargetUpdated( @@ -131,6 +127,12 @@ AccessibilityPanel.prototype = { shutdown: this.onLifecycleEvent, }); + // Force refresh to render the UI and wait for the INITIALIZED event. + const onInitialized = this.panelWin.once(EVENTS.INITIALIZED); + this.shouldRefresh = true; + this.refresh(); + await onInitialized; + resolver(this); return this._opening; }, diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_relation_navigation.js b/devtools/client/accessibility/test/browser/browser_accessibility_relation_navigation.js index 34ae44cdb911..ab6e0fef6eb4 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_relation_navigation.js +++ b/devtools/client/accessibility/test/browser/browser_accessibility_relation_navigation.js @@ -42,7 +42,16 @@ const tests = [ keyboardShortcut: "", childCount: 2, indexInParent: 0, - states: ["readonly", "focusable", "opaque", "enabled", "sensitive"], + states: [ + // The focused state is an outdated state, since the toolbox should now + // have the focus and not the content page. See Bug 1702709. + "focused", + "readonly", + "focusable", + "opaque", + "enabled", + "sensitive", + ], }, }, }, diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_sidebar.js b/devtools/client/accessibility/test/browser/browser_accessibility_sidebar.js index b08488c38e4a..e7c2795ced81 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_sidebar.js +++ b/devtools/client/accessibility/test/browser/browser_accessibility_sidebar.js @@ -33,7 +33,16 @@ const tests = [ keyboardShortcut: "", childCount: 0, indexInParent: 0, - states: ["readonly", "focusable", "opaque", "enabled", "sensitive"], + states: [ + // The focused state is an outdated state, since the toolbox should now + // have the focus and not the content page. See Bug 1702709. + "focused", + "readonly", + "focusable", + "opaque", + "enabled", + "sensitive", + ], }, }, }, diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation.js b/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation.js index 5d32fd520216..d98c3d8e35dc 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation.js +++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation.js @@ -42,7 +42,16 @@ const tests = [ keyboardShortcut: "", childCount: 2, indexInParent: 0, - states: ["readonly", "focusable", "opaque", "enabled", "sensitive"], + states: [ + // The focused state is an outdated state, since the toolbox should now + // have the focus and not the content page. See Bug 1702709. + "focused", + "readonly", + "focusable", + "opaque", + "enabled", + "sensitive", + ], }, }, }, diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation_oop.js b/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation_oop.js index d712173a27de..dc710c82e4f2 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation_oop.js +++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_nagivation_oop.js @@ -34,7 +34,16 @@ const tests = [ keyboardShortcut: "", childCount: 1, indexInParent: 0, - states: ["readonly", "focusable", "opaque", "enabled", "sensitive"], + states: [ + // The focused state is an outdated state, since the toolbox should now + // have the focus and not the content page. See Bug 1702709. + "focused", + "readonly", + "focusable", + "opaque", + "enabled", + "sensitive", + ], }, }, }, diff --git a/testing/talos/talos/tests/devtools/addon/content/tests/accessibility/accessibility-helpers.js b/testing/talos/talos/tests/devtools/addon/content/tests/accessibility/accessibility-helpers.js index 5b453b7af01b..95cbcaa50e8f 100644 --- a/testing/talos/talos/tests/devtools/addon/content/tests/accessibility/accessibility-helpers.js +++ b/testing/talos/talos/tests/devtools/addon/content/tests/accessibility/accessibility-helpers.js @@ -25,11 +25,7 @@ exports.shutdownAccessibilityService = function() { }; exports.openAccessibilityAndLog = function(label) { - return openToolboxAndLog( - `${label}.accessibility`, - "accessibility", - (toolbox, panel) => panel.panelWin.once(INITIALIZED_EVENT) - ); + return openToolboxAndLog(`${label}.accessibility`, "accessibility"); }; exports.reloadAccessibilityAndLog = async function(label, toolbox) {