Bug 1226898 - Make autoscroll distance depend on the document height r=pbro

--HG-- rename : devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll.js => devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll_01.js rename : devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll.html => devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll_02.html
2024-10-22 09:45:41 +00:00 · 2016-07-05 11:52:25 -07:00 · 2016-07-05 11:52:25 -07:00 · 84f5e124c1
commit 84f5e124c1
parent dbbc213968
8 changed files with 244 additions and 56 deletions
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@ -15,10 +15,14 @@ const DEFAULT_MAX_CHILDREN = 100;
 const COLLAPSE_DATA_URL_REGEX = /^data.+base64/;
 const COLLAPSE_DATA_URL_LENGTH = 60;
 const NEW_SELECTION_HIGHLIGHTER_TIMER = 1000;
-const DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE = 50;
-const DRAG_DROP_MIN_AUTOSCROLL_SPEED = 5;
-const DRAG_DROP_MAX_AUTOSCROLL_SPEED = 15;
+const DRAG_DROP_AUTOSCROLL_EDGE_MAX_DISTANCE = 50;
+const DRAG_DROP_AUTOSCROLL_EDGE_RATIO = 0.1;
+const DRAG_DROP_MIN_AUTOSCROLL_SPEED = 2;
+const DRAG_DROP_MAX_AUTOSCROLL_SPEED = 8;
 const DRAG_DROP_MIN_INITIAL_DISTANCE = 10;
+const DRAG_DROP_HEIGHT_TO_SPEED = 500;
+const DRAG_DROP_HEIGHT_TO_SPEED_MIN = 0.5;
+const DRAG_DROP_HEIGHT_TO_SPEED_MAX = 1;
 const AUTOCOMPLETE_POPUP_PANEL_ID = "markupview_autoCompletePopup";
 const ATTR_COLLAPSE_ENABLED_PREF = "devtools.markup.collapseAttributes";
 const ATTR_COLLAPSE_LENGTH_PREF = "devtools.markup.collapseAttributeLength";
@ -252,39 +256,60 @@ MarkupView.prototype = {
  _autoScroll: function (event) {
    let docEl = this.doc.documentElement;

-    if (this._autoScrollInterval) {
-      clearInterval(this._autoScrollInterval);
+    if (this._autoScrollAnimationFrame) {
+      this.win.cancelAnimationFrame(this._autoScrollAnimationFrame);
    }

    // Auto-scroll when the mouse approaches top/bottom edge.
    let fromBottom = docEl.clientHeight - event.pageY + this.win.scrollY;
    let fromTop = event.pageY - this.win.scrollY;
+    let edgeDistance = Math.min(DRAG_DROP_AUTOSCROLL_EDGE_MAX_DISTANCE,
+           docEl.clientHeight * DRAG_DROP_AUTOSCROLL_EDGE_RATIO);

-    if (fromBottom <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) {
-      // Map our distance from 0-50 to 5-15 range so the speed is kept in a
-      // range not too fast, not too slow.
+    // The smaller the screen, the slower the movement.
+    let heightToSpeedRatio =
+      Math.max(DRAG_DROP_HEIGHT_TO_SPEED_MIN,
+        Math.min(DRAG_DROP_HEIGHT_TO_SPEED_MAX,
+          docEl.clientHeight / DRAG_DROP_HEIGHT_TO_SPEED));
+
+    if (fromBottom <= edgeDistance) {
+      // Map our distance range to a speed range so that the speed is not too
+      // fast or too slow.
      let speed = map(
        fromBottom,
-        0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE,
+        0, edgeDistance,
        DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED);

-      this._autoScrollInterval = setInterval(() => {
-        docEl.scrollTop -= speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED;
-      }, 0);
+      this._runUpdateLoop(() => {
+        docEl.scrollTop -= heightToSpeedRatio *
+          (speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED);
+      });
    }

-    if (fromTop <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) {
+    if (fromTop <= edgeDistance) {
      let speed = map(
        fromTop,
-        0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE,
+        0, edgeDistance,
        DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED);

-      this._autoScrollInterval = setInterval(() => {
-        docEl.scrollTop += speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED;
-      }, 0);
+      this._runUpdateLoop(() => {
+        docEl.scrollTop += heightToSpeedRatio *
+          (speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED);
+      });
    }
  },

+  /**
+   * Run a loop on the requestAnimationFrame.
+   */
+  _runUpdateLoop: function (update) {
+    let loop = () => {
+      update();
+      this._autoScrollAnimationFrame = this.win.requestAnimationFrame(loop);
+    };
+    loop();
+  },
+
  _onMouseClick: function (event) {
    // From the target passed here, let's find the parent MarkupContainer
    // and ask it if the tooltip should be shown
@ -309,8 +334,8 @@ MarkupView.prototype = {
  _onMouseUp: function () {
    this.indicateDropTarget(null);
    this.indicateDragTarget(null);
-    if (this._autoScrollInterval) {
-      clearInterval(this._autoScrollInterval);
+    if (this._autoScrollAnimationFrame) {
+      this.win.cancelAnimationFrame(this._autoScrollAnimationFrame);
    }
  },

@ -336,8 +361,8 @@ MarkupView.prototype = {

    this.indicateDropTarget(null);
    this.indicateDragTarget(null);
-    if (this._autoScrollInterval) {
-      clearInterval(this._autoScrollInterval);
+    if (this._autoScrollAnimationFrame) {
+      this.win.cancelAnimationFrame(this._autoScrollAnimationFrame);
    }
  },

@ -363,8 +388,8 @@ MarkupView.prototype = {
  },

  _onMouseLeave: function () {
-    if (this._autoScrollInterval) {
-      clearInterval(this._autoScrollInterval);
+    if (this._autoScrollAnimationFrame) {
+      this.win.cancelAnimationFrame(this._autoScrollAnimationFrame);
    }
    if (this.isDragging) {
      return;
@ -1884,6 +1909,7 @@ MarkupContainer.prototype = {
    this.undo = this.markup.undo;
    this.win = this.markup._frame.contentWindow;
    this.id = "treeitem-" + markupContainerID++;
+    this.htmlElt = this.win.document.documentElement;

    // The template will fill the following properties
    this.elt = null;
@ -2185,10 +2211,12 @@ MarkupContainer.prototype = {
    this.tagLine.setAttribute("aria-grabbed", isDragging);

    if (isDragging) {
+      this.htmlElt.classList.add("dragging");
      this.elt.classList.add("dragging");
      this.markup.doc.body.classList.add("dragging");
      rootElt.setAttribute("aria-dropeffect", "move");
    } else {
+      this.htmlElt.classList.remove("dragging");
      this.elt.classList.remove("dragging");
      this.markup.doc.body.classList.remove("dragging");
      rootElt.setAttribute("aria-dropeffect", "none");
@ -2525,6 +2553,7 @@ MarkupContainer.prototype = {
    }

    this.win = null;
+    this.htmlElt = null;

    if (this.expander) {
      this.expander.removeEventListener("click", this._onToggle, false);
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@ -5,7 +5,8 @@ support-files =
  actor_events_form.js
  doc_markup_anonymous.html
  doc_markup_dragdrop.html
-  doc_markup_dragdrop_autoscroll.html
+  doc_markup_dragdrop_autoscroll_01.html
+  doc_markup_dragdrop_autoscroll_02.html
  doc_markup_edit.html
  doc_markup_events1.html
  doc_markup_events2.html
@ -65,7 +66,8 @@ subsuite = clipboard
 [browser_markup_css_completion_style_attribute_01.js]
 [browser_markup_css_completion_style_attribute_02.js]
 [browser_markup_css_completion_style_attribute_03.js]
-[browser_markup_dragdrop_autoscroll.js]
+[browser_markup_dragdrop_autoscroll_01.js]
+[browser_markup_dragdrop_autoscroll_02.js]
 [browser_markup_dragdrop_distance.js]
 [browser_markup_dragdrop_draggable.js]
 [browser_markup_dragdrop_dragRootNode.js]
--- a/devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll_01.js
+++ b/devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll_01.js
@ -5,11 +5,15 @@
 "use strict";

 // Test that dragging a node near the top or bottom edge of the markup-view
-// auto-scrolls the view.
+// auto-scrolls the view on a large toolbox.

-const TEST_URL = URL_ROOT + "doc_markup_dragdrop_autoscroll.html";
+const TEST_URL = URL_ROOT + "doc_markup_dragdrop_autoscroll_01.html";

 add_task(function* () {
+  // Set the toolbox as large as it would get. The toolbox automatically shrinks
+  // to not overflow to window.
+  yield pushPref("devtools.toolbox.footer.height", 10000);
+
  let {inspector} = yield openInspectorForURL(TEST_URL);
  let markup = inspector.markup;
  let viewHeight = markup.doc.documentElement.clientHeight;
@ -18,7 +22,7 @@ add_task(function* () {
  markup.isDragging = true;

  info("Simulate a mousemove on the view, at the bottom, and expect scrolling");
-  let onScrolled = waitForScrollStop(markup);
+  let onScrolled = waitForScrollStop(markup.doc);

  markup._onMouseMove({
    preventDefault: () => {},
@ -30,7 +34,7 @@ add_task(function* () {
  ok(bottomScrollPos > 0, "The view was scrolled down");

  info("Simulate a mousemove at the top and expect more scrolling");
-  onScrolled = waitForScrollStop(markup);
+  onScrolled = waitForScrollStop(markup.doc);

  markup._onMouseMove({
    preventDefault: () => {},
@ -45,29 +49,3 @@ add_task(function* () {
  info("Simulate a mouseup to stop dragging");
  markup._onMouseUp();
 });
-
-/**
- * Waits until the element has not scrolled for 30 consecutive frames.
- */
-function* waitForScrollStop(markup) {
-  let el = markup.doc.documentElement;
-  let win = markup.doc.defaultView;
-  let lastScrollTop = el.scrollTop;
-  let stopFrameCount = 0;
-  while (stopFrameCount < 30) {
-    // Wait for a frame.
-    yield new Promise(resolve => win.requestAnimationFrame(resolve));
-
-    // Check if the element has scrolled.
-    if (lastScrollTop == el.scrollTop) {
-      // No scrolling since the last frame.
-      stopFrameCount++;
-    } else {
-      // The element has scrolled. Reset the frame counter.
-      stopFrameCount = 0;
-      lastScrollTop = el.scrollTop;
-    }
-  }
-
-  return lastScrollTop;
-}
--- a/devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll_02.js
+++ b/devtools/client/inspector/markup/test/browser_markup_dragdrop_autoscroll_02.js
@ -0,0 +1,50 @@
+/* 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 dragging a node near the top or bottom edge of the markup-view
+// auto-scrolls the view on a small toolbox.
+
+const TEST_URL = URL_ROOT + "doc_markup_dragdrop_autoscroll_02.html";
+
+add_task(function* () {
+  // Set the toolbox to very small in size.
+  yield pushPref("devtools.toolbox.footer.height", 150);
+
+  let {inspector} = yield openInspectorForURL(TEST_URL);
+  let markup = inspector.markup;
+  let viewHeight = markup.doc.documentElement.clientHeight;
+
+  info("Pretend the markup-view is dragging");
+  markup.isDragging = true;
+
+  info("Simulate a mousemove on the view, at the bottom, and expect scrolling");
+  let onScrolled = waitForScrollStop(markup.doc);
+
+  markup._onMouseMove({
+    preventDefault: () => {},
+    target: markup.doc.body,
+    pageY: viewHeight
+  });
+
+  let bottomScrollPos = yield onScrolled;
+  ok(bottomScrollPos > 0, "The view was scrolled down");
+
+  info("Simulate a mousemove at the top and expect more scrolling");
+  onScrolled = waitForScrollStop(markup.doc);
+
+  markup._onMouseMove({
+    preventDefault: () => {},
+    target: markup.doc.body,
+    pageY: 0
+  });
+
+  let topScrollPos = yield onScrolled;
+  ok(topScrollPos < bottomScrollPos, "The view was scrolled up");
+  is(topScrollPos, 0, "The view was scrolled up to the top");
+
+  info("Simulate a mouseup to stop dragging");
+  markup._onMouseUp();
+});
--- a/devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll_01.html
+++ b/devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll_01.html
@ -0,0 +1,87 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=858038
+https://bugzilla.mozilla.org/show_bug.cgi?id=1226898
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 858038 and 1226898 - Autoscroll</title>
+</head>
+<body>
+  <div id="first"></div>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=858038">Mozilla Bug 858038</a>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1226898">Mozilla Bug 1226898</a>
+  <p id="display">Test</p>
+  <div id="content" style="display: none">
+
+  </div>
+
+  <!-- Make sure the markup-view has enough nodes shown by default that it has a scrollbar -->
+
+  <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>
+  <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>
+  <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>
+</body>
+</html>
--- a/devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll_02.html
+++ b/devtools/client/inspector/markup/test/doc_markup_dragdrop_autoscroll_02.html
@ -2,14 +2,16 @@
 <html>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=858038
+https://bugzilla.mozilla.org/show_bug.cgi?id=1226898
 -->
 <head>
  <meta charset="utf-8">
-  <title>Test for Bug 858038 - Autoscroll</title>
+  <title>Test for Bug 858038 and 1226898 - Autoscroll</title>
 </head>
 <body>
  <div id="first"></div>
  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=858038">Mozilla Bug 858038</a>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1226898">Mozilla Bug 1226898</a>
  <p id="display">Test</p>
  <div id="content" style="display: none">

@ -47,5 +49,15 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=858038
  <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>
 </body>
 </html>
--- a/devtools/client/inspector/markup/test/head.js
+++ b/devtools/client/inspector/markup/test/head.js
@ -584,3 +584,29 @@ function* simulateNodeDragAndDrop(inspector, selector, xOffset, yOffset) {
  yield simulateNodeDrag(inspector, selector, xOffset, yOffset);
  yield simulateNodeDrop(inspector, selector);
 }
+
+/**
+ * Waits until the element has not scrolled for 30 consecutive frames.
+ */
+function* waitForScrollStop(doc) {
+  let el = doc.documentElement;
+  let win = doc.defaultView;
+  let lastScrollTop = el.scrollTop;
+  let stopFrameCount = 0;
+  while (stopFrameCount < 30) {
+    // Wait for a frame.
+    yield new Promise(resolve => win.requestAnimationFrame(resolve));
+
+    // Check if the element has scrolled.
+    if (lastScrollTop == el.scrollTop) {
+      // No scrolling since the last frame.
+      stopFrameCount++;
+    } else {
+      // The element has scrolled. Reset the frame counter.
+      stopFrameCount = 0;
+      lastScrollTop = el.scrollTop;
+    }
+  }
+
+  return lastScrollTop;
+}
--- a/devtools/client/themes/markup.css
+++ b/devtools/client/themes/markup.css
@ -36,6 +36,10 @@ body {
  background: none;
 }

+html.dragging {
+  overflow-x: hidden;
+}
+
 body.dragging .tag-line {
  cursor: grabbing;
 }