From f9ad45c76ba50bdee54bebd14e6625ae14d4d085 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Thu, 28 Jan 2021 15:33:47 +0000 Subject: [PATCH] Bug 1686241 - [devtools] Stop selecting the node front when expanding the markup view container r=nchevobbe Depends on D103051. Other browser devtools do not select the node on expand. Selecting the node can be quite slow when if many CSS rules apply to it. This should allow exploring the tree of complex pages with less slowdowns. An existing test is updated to assert the selected state of the node container before/after expanding it. Differential Revision: https://phabricator.services.mozilla.com/D103052 --- .../markup/test/browser_markup_toggle_01.js | 7 ++-- .../markup/test/browser_markup_toggle_03.js | 4 +-- .../browser_markup_toggle_closing_tag_line.js | 6 ++-- .../markup/views/markup-container.js | 7 ++-- devtools/client/inspector/test/head.js | 32 ++----------------- 5 files changed, 18 insertions(+), 38 deletions(-) diff --git a/devtools/client/inspector/markup/test/browser_markup_toggle_01.js b/devtools/client/inspector/markup/test/browser_markup_toggle_01.js index 85246f7e773f..5e30ebe856cd 100644 --- a/devtools/client/inspector/markup/test/browser_markup_toggle_01.js +++ b/devtools/client/inspector/markup/test/browser_markup_toggle_01.js @@ -21,9 +21,11 @@ add_task(async function() { ok(!container.mustExpand, "UL element !mustExpand"); ok(container.canExpand, "UL element canExpand"); is(container.expander.style.visibility, "visible", "HTML twisty is visible"); + ok(!container.selected, "UL container is not selected"); info("Clicking on the UL parent expander, and waiting for children"); - await expandContainerByClick(inspector, container); + await toggleContainerByClick(inspector, container); + ok(!container.selected, "UL container is still not selected after expand"); info("Checking that child LI elements have been created"); let numLi = await testActor.getNumberOfElementMatches("li"); @@ -37,7 +39,8 @@ add_task(async function() { ok(container.expanded, "Parent UL container is expanded"); info("Clicking again on the UL expander"); - collapseContainerByClick(inspector, container); + await toggleContainerByClick(inspector, container); + ok(!container.selected, "UL container is still not selected after collapse"); info("Checking that child LI elements have been hidden"); numLi = await testActor.getNumberOfElementMatches("li"); diff --git a/devtools/client/inspector/markup/test/browser_markup_toggle_03.js b/devtools/client/inspector/markup/test/browser_markup_toggle_03.js index 3cc7ea53a30b..1b53efb817bd 100644 --- a/devtools/client/inspector/markup/test/browser_markup_toggle_03.js +++ b/devtools/client/inspector/markup/test/browser_markup_toggle_03.js @@ -15,7 +15,7 @@ add_task(async function() { const container = await getContainerForSelector("ul", inspector); info("Alt-clicking on collapsed expander should expand all children"); - await expandContainerByClick(inspector, container, { altKey: true }); + await toggleContainerByClick(inspector, container, { altKey: true }); info("Checking that all nodes exist and are expanded"); let nodeFronts = await getNodeFronts(inspector); @@ -29,7 +29,7 @@ add_task(async function() { } info("Alt-clicking on expanded expander should collapse all children"); - await collapseContainerByClick(inspector, container, { altKey: true }); + await toggleContainerByClick(inspector, container, { altKey: true }); info("Checking that all nodes are collapsed"); nodeFronts = await getNodeFronts(inspector); diff --git a/devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js b/devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js index c9cc21e20d01..945cb92d562c 100644 --- a/devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js +++ b/devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js @@ -16,7 +16,7 @@ add_task(async function() { info("Getting the container for .outer-div parent element"); let container = await getContainerForSelector(".outer-div", inspector); - await expandContainerByClick(inspector, container); + await toggleContainerByClick(inspector, container); let closeTagLine = container.closeTagLine; ok( @@ -26,7 +26,7 @@ add_task(async function() { info("Expand the iframe element"); container = await getContainerForSelector("iframe", inspector); - await expandContainerByClick(inspector, container); + await toggleContainerByClick(inspector, container); ok(container.expanded, "iframe is expanded"); closeTagLine = container.closeTagLine; ok( @@ -45,7 +45,7 @@ add_task(async function() { info("Expand the iframe's #document node element"); container = getContainerForNodeFront(documentFront, inspector); - await expandContainerByClick(inspector, container); + await toggleContainerByClick(inspector, container); ok(container.expanded, "#document is expanded"); ok( !container.closeTagLine, diff --git a/devtools/client/inspector/markup/views/markup-container.js b/devtools/client/inspector/markup/views/markup-container.js index 3b32e58d4efa..446cd0d05e91 100644 --- a/devtools/client/inspector/markup/views/markup-container.js +++ b/devtools/client/inspector/markup/views/markup-container.js @@ -545,6 +545,11 @@ MarkupContainer.prototype = { return; } + // Bail out when clicking on arrow expanders to avoid selecting the row. + if (target.classList.contains("expander")) { + return; + } + // target is the MarkupContainer itself. this.hovered = false; this.markup.navigate(this); @@ -808,8 +813,6 @@ MarkupContainer.prototype = { * Whether all descendants should also be expanded/collapsed */ expandContainer: function(applyToDescendants) { - this.markup.navigate(this); - if (this.hasChildren) { this.markup.setNodeExpanded( this.node, diff --git a/devtools/client/inspector/test/head.js b/devtools/client/inspector/test/head.js index 0615a3d085a1..2c7adc8ed607 100644 --- a/devtools/client/inspector/test/head.js +++ b/devtools/client/inspector/test/head.js @@ -1171,8 +1171,8 @@ async function expandContainer(inspector, container) { } /** - * Expand the provided markup container by clicking on the expand arrow and waiting for - * inspector and children to update. Similar to expandContainer helper, but this method + * Toggle the provided markup container by clicking on the expand arrow and waiting for + * children to update. Similar to expandContainer helper, but this method * uses a click rather than programatically calling expandNode(). * * @param {InspectorPanel} inspector @@ -1183,37 +1183,11 @@ async function expandContainer(inspector, container) { * options.altKey {Boolean} Use the altKey modifier, to recursively apply * the action to all the children of the container. */ -async function expandContainerByClick( +async function toggleContainerByClick( inspector, container, { altKey = false } = {} ) { - const onUpdated = inspector.once("inspector-updated"); - EventUtils.synthesizeMouseAtCenter( - container.expander, - { - altKey, - }, - inspector.markup.doc.defaultView - ); - - // Wait for any pending children updates - await waitForMultipleChildrenUpdates(inspector); - - // Wait for inspector-updated triggered by selecting the node. - await onUpdated; -} - -/** - * Collapse the provided markup container. See expandContainerByClick. - */ -async function collapseContainerByClick( - inspector, - container, - { altKey = false } = {} -) { - // No need to wait, this is a local, synchronous operation where nodes are - // only hidden from the view, not destroyed EventUtils.synthesizeMouseAtCenter( container.expander, {