Bug 1460848 - Always scroll to container element when clicking on reveal;r=bgrins
☠☠ backed out by da340330e931 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 25 Jun 2018 17:59:04 +0200
changeset 478987 ed0176b3adac9dedc614993e464abf8d42f5534f
parent 478986 86fcc230d64d74329c2e517c300ec3e5289c2a32
child 478988 da340330e931fe0f604fdca2513e0f60ba125675
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1460848
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1460848 - Always scroll to container element when clicking on reveal;r=bgrins MozReview-Commit-ID: 6KY8EBYX0uD
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_shadowdom_clickreveal.js
devtools/client/inspector/markup/test/browser_markup_shadowdom_clickreveal_scroll.js
devtools/client/inspector/markup/test/head.js
devtools/client/inspector/markup/views/slotted-node-container.js
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -161,16 +161,17 @@ skip-if = verify
 [browser_markup_node_not_displayed_02.js]
 [browser_markup_pagesize_01.js]
 [browser_markup_pagesize_02.js]
 [browser_markup_remove_xul_attributes.js]
 skip-if = e10s # Bug 1036409 - The last selected node isn't reselected
 [browser_markup_search_01.js]
 [browser_markup_shadowdom.js]
 [browser_markup_shadowdom_clickreveal.js]
+[browser_markup_shadowdom_clickreveal_scroll.js]
 [browser_markup_shadowdom_delete.js]
 [browser_markup_shadowdom_maxchildren.js]
 [browser_markup_shadowdom_mutations_shadow.js]
 [browser_markup_shadowdom_navigation.js]
 [browser_markup_shadowdom_noslot.js]
 [browser_markup_shadowdom_slotupdate.js]
 [browser_markup_tag_delete_whitespace_node.js]
 [browser_markup_tag_edit_01.js]
--- a/devtools/client/inspector/markup/test/browser_markup_shadowdom_clickreveal.js
+++ b/devtools/client/inspector/markup/test/browser_markup_shadowdom_clickreveal.js
@@ -66,21 +66,8 @@ async function checkRevealLink(inspector
   info("Click on the reveal link and wait for the new node to be selected");
   await clickOnRevealLink(inspector, slottedContainer);
   const selectedFront = inspector.selection.nodeFront;
   is(selectedFront, node, "The same node front is still selected");
   ok(!inspector.selection.isSlotted(), "The selection is not the slotted version");
   ok(!inspector.markup.getSelectedContainer().isSlotted(),
     "The selected container is not slotted");
 }
-
-async function clickOnRevealLink(inspector, container) {
-  const onSelection = inspector.selection.once("new-node-front");
-  const revealLink = container.elt.querySelector(".reveal-link");
-  const tagline = revealLink.closest(".tag-line");
-  const win = inspector.markup.doc.defaultView;
-
-  // First send a mouseover on the tagline to force the link to be displayed.
-  EventUtils.synthesizeMouseAtCenter(tagline, {type: "mouseover"}, win);
-  EventUtils.synthesizeMouseAtCenter(revealLink, {}, win);
-
-  await onSelection;
-}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/browser_markup_shadowdom_clickreveal_scroll.js
@@ -0,0 +1,85 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that clicking on "reveal" always scrolls the view to show the real container, even
+// if the node is already selected.
+
+const TEST_URL = `data:text/html;charset=utf-8,
+  <test-component>
+    <div slot="slot1" id="el1">slot1 content</div>
+  </test-component>
+
+  <script>
+    'use strict';
+    customElements.define('test-component', class extends HTMLElement {
+      constructor() {
+        super();
+        let shadowRoot = this.attachShadow({mode: 'open'});
+        shadowRoot.innerHTML = \`
+          <slot name="slot1"></slot>
+          <div></div><div></div><div></div><div></div><div></div><div></div>
+          <div></div><div></div><div></div><div></div><div></div><div></div>
+          <div></div><div></div><div></div><div></div><div></div><div></div>
+          <div></div><div></div><div></div><div></div><div></div><div></div>
+          <!-- adding some nodes to make sure the slotted container and the real container
+           require scrolling -->
+        \`;
+      }
+    });
+  </script>`;
+
+add_task(async function() {
+  await enableWebComponents();
+
+  const {inspector} = await openInspectorForURL(TEST_URL);
+  const {markup} = inspector;
+
+  info("Find and expand the test-component shadow DOM host.");
+  const hostFront = await getNodeFront("test-component", inspector);
+  const hostContainer = markup.getContainer(hostFront);
+  await expandContainer(inspector, hostContainer);
+
+  info("Expand the shadow root");
+  const shadowRootContainer = hostContainer.getChildContainers()[0];
+  await expandContainer(inspector, shadowRootContainer);
+
+  info("Expand the slot");
+  const slotContainer = shadowRootContainer.getChildContainers()[0];
+  await expandContainer(inspector, slotContainer);
+
+  const slotChildContainers = slotContainer.getChildContainers();
+  is(slotChildContainers.length, 1, "Expecting 1 slotted child");
+
+  const slottedContainer = slotChildContainers[0];
+  const realContainer = inspector.markup.getContainer(slottedContainer.node);
+  const slottedElement = slottedContainer.elt;
+  const realElement = realContainer.elt;
+
+  info("Click on the reveal link");
+  await clickOnRevealLink(inspector, slottedContainer);
+  // "new-node-front" will also trigger the scroll, so make sure we are testing after
+  // the scroll was performed.
+  await waitUntil(() => isScrolledOut(slottedElement));
+  is(isScrolledOut(slottedElement), true, "slotted element is scrolled out");
+  is(isScrolledOut(realElement), false, "real element is not scrolled out");
+
+  info("Scroll back to see the slotted element");
+  slottedElement.scrollIntoView();
+  is(isScrolledOut(slottedElement), false, "slotted element is not scrolled out");
+  is(isScrolledOut(realElement), true, "real element is scrolled out");
+
+  info("Click on the reveal link again");
+  await clickOnRevealLink(inspector, slottedContainer);
+  await waitUntil(() => isScrolledOut(slottedElement));
+  is(isScrolledOut(slottedElement), true, "slotted element is scrolled out");
+  is(isScrolledOut(realElement), false, "real element is not scrolled out");
+});
+
+function isScrolledOut(element) {
+  const win = element.ownerGlobal;
+  const rect = element.getBoundingClientRect();
+  return rect.top < 0 || (rect.top + rect.height) > win.innerHeight;
+}
--- a/devtools/client/inspector/markup/test/head.js
+++ b/devtools/client/inspector/markup/test/head.js
@@ -710,8 +710,24 @@ function waitForNMutations(inspector, ty
       if (receivedMutations == count) {
         inspector.off("markupmutation", onMutation);
         resolve();
       }
     });
   });
 }
 
+/**
+ * Click on the reveal link the provided slotted container.
+ * Will resolve when selection emits "new-node-front".
+ */
+async function clickOnRevealLink(inspector, container) {
+  const onSelection = inspector.selection.once("new-node-front");
+  const revealLink = container.elt.querySelector(".reveal-link");
+  const tagline = revealLink.closest(".tag-line");
+  const win = inspector.markup.doc.defaultView;
+
+  // First send a mouseover on the tagline to force the link to be displayed.
+  EventUtils.synthesizeMouseAtCenter(tagline, {type: "mouseover"}, win);
+  EventUtils.synthesizeMouseAtCenter(revealLink, {}, win);
+
+  await onSelection;
+}
--- a/devtools/client/inspector/markup/views/slotted-node-container.js
+++ b/devtools/client/inspector/markup/views/slotted-node-container.js
@@ -35,20 +35,19 @@ SlottedNodeContainer.prototype = extend(
   },
 
   onContainerClick: async function(event) {
     if (!event.target.classList.contains("reveal-link")) {
       return;
     }
 
     const selection = this.markup.inspector.selection;
-    if (selection.nodeFront != this.node || selection.isSlotted()) {
-      const reason = "reveal-from-slot";
-      this.markup.inspector.selection.setNodeFront(this.node, { reason });
-    }
+    this.markup.inspector.selection.setNodeFront(this.node, {
+      reason: "reveal-from-slot"
+    });
   },
 
   isDraggable: function() {
     return false;
   },
 
   isSlotted: function() {
     return true;