Bug 1460848 - Always scroll to container element when clicking on reveal;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 25 Jun 2018 17:59:04 +0200
changeset 811254 575a73f7b28db217e7f1c59c05b01a4cd23eddb6
parent 810775 3f37819f8305e6c373d957f93da2ff7d63c38ce5
push id114247
push userjdescottes@mozilla.com
push dateWed, 27 Jun 2018 09:37:33 +0000
reviewersbgrins
bugs1460848
milestone62.0a1
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
@@ -767,8 +767,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
@@ -34,21 +34,19 @@ SlottedNodeContainer.prototype = extend(
     event.stopPropagation();
   },
 
   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;