Make scrolling happen relative to the main content frame

Summary: Fixes T7033. When we've reframed the main page content we need to scroll relative to the containing frame, not relative to the window.

Test Plan:
In Safari, Chrome and Firefox, used j/k/J/K keys to navigate diff content.

Tried some other scroll-based beahviors, like jump-to-anchors.

(It looks like the highlighting reticle got slightly derped a while ago, but it's still functional, so I didn't mess with it.)

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7033

Differential Revision: https://secure.phabricator.com/D11490

src/view/page/PhabricatorStandardPageView.php

@@ -344,6 +344,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       'scrollbar',
       array(
         'nodeID' => 'phabricator-standard-page',
+        'isMainContent' => true,
       ));
 
     $main_page = phutil_tag(

webroot/rsrc/externals/javelin/lib/DOM.js

@@ -336,6 +336,8 @@ JX.install('DOM', {
     _autoid : 0,
     _uniqid : 0,
     _metrics : {},
+    _frameNode: null,
+    _contentNode: null,
 
 
 /* -(  Changing DOM Content  )----------------------------------------------- */
@@ -935,14 +937,71 @@ JX.install('DOM', {
     },
 
 
+    /**
+     * Set specific nodes as content and frame nodes for the document.
+     *
+     * This will cause @{method:scrollTo} and @{method:scrollToPosition} to
+     * affect the given frame node instead of the window. This is useful if the
+     * page content is broken into multiple panels which scroll independently.
+     *
+     * Normally, both nodes are the document body.
+     *
+     * @task view
+     * @param Node Node to set as the scroll frame.
+     * @param Node Node to set as the content frame.
+     * @return void
+     */
+    setContentFrame: function(frame_node, content_node) {
+      JX.DOM._frameNode = frame_node;
+      JX.DOM._contentNode = content_node;
+    },
+
+
+    /**
+     * Get the current content frame, or `document.body` if one has not been
+     * set.
+     *
+     * @task view
+     * @return Node The node which frames the main page content.
+     * @return void
+     */
+    getContentFrame: function() {
+      return JX.DOM._contentNode || document.body;
+    },
+
     /**
      * Scroll to the position of an element in the document.
+     *
+     * If @{method:setContentFrame} has been used to set a frame, that node is
+     * scrolled.
+     *
      * @task view
      * @param Node Node to move document scroll position to, if possible.
      * @return void
      */
     scrollTo : function(node) {
-      window.scrollTo(0, JX.$V(node).y);
+      JX.DOM._scrollToPosition(0, JX.$V(node).y);
+    },
+
+    /**
+     * Scroll to a specific position in the document.
+     *
+     * If @{method:setContentFrame} has been used to set a frame, that node is
+     * scrolled.
+     *
+     * @task view
+     * @param int X position, in pixels.
+     * @param int Y position, in pixels.
+     * @return void
+     */
+    scrollToPosition: function(x, y) {
+      var self = JX.DOM;
+      if (self._frameNode) {
+        self._frameNode.scrollLeft = x;
+        self._frameNode.scrollTop = y;
+      } else {
+        window.scrollTo(x, y);
+      }
     },
 
     _getAutoID : function(node) {

webroot/rsrc/externals/javelin/lib/Scrollbar.js

@@ -25,6 +25,8 @@
 JX.install('Scrollbar', {
 
   construct: function(frame) {
+    this._frame = frame;
+
     // Before doing anything, check if the scrollbar control has a measurable
     // width. If it doesn't, we're already in an environment with an aesthetic
     // scrollbar (like Safari on OSX with no mouse connected, or an iPhone)
@@ -60,7 +62,6 @@ JX.install('Scrollbar', {
     var viewport = JX.$N('div', {className: 'jx-scrollbar-viewport'}, content);
     JX.DOM.appendContent(frame, viewport);
 
-    this._frame = frame;
     this._viewport = viewport;
     this._content = content;
 
@@ -134,6 +135,26 @@ JX.install('Scrollbar', {
     _scrollOrigin: null,
 
 
+    /**
+     * Mark this content as the scroll frame.
+     *
+     * This changes the behavior of the @{class:JX.DOM} scroll functions so the
+     * continue to work properly if the main page content is reframed to scroll
+     * independently.
+     */
+    setAsScrollFrame: function() {
+      if (this._viewport) {
+        // If we activated the scrollbar, the viewport and content nodes become
+        // the new scroll and content frames.
+        JX.DOM.setContentFrame(this._viewport, this._content);
+      } else {
+        // Otherwise, the unaltered content frame is both the scroll frame and
+        // content frame.
+        JX.DOM.setContentFrame(this._frame, this._frame);
+      }
+    },
+
+
     /**
      * After the user scrolls the page, show the scrollbar to give them
      * feedback about their position.

webroot/rsrc/externals/javelin/lib/Workflow.js

@@ -204,7 +204,7 @@ JX.install('Workflow', {
         // The `focus()` call may have scrolled the window. Scroll it back to
         // where it was before -- we want to focus the control, but not adjust
         // the scroll position.
-        window.scrollTo(s.x, s.y);
+        JX.DOM.scrollToPosition(s.x, s.y);
 
       } else if (this.getHandler()) {
         this.getHandler()(r);

webroot/rsrc/js/application/differential/ChangesetViewManager.js

@@ -257,7 +257,7 @@ JX.install('ChangesetViewManager', {
           if (near_bot || above_mid) {
             // Figure out how much taller the document got.
             var delta = (JX.Vector.getDocument().y - old_dim.y);
-            window.scrollTo(old_pos.x, old_pos.y + delta);
+            JX.DOM.scrollToPosition(old_pos.x, old_pos.y + delta);
           }
         }
         this._stabilize = false;

webroot/rsrc/js/application/diffusion/behavior-jump-to.js

@@ -8,7 +8,7 @@
 JX.behavior('diffusion-jump-to', function(config) {
 
   setTimeout(function() {
-    window.scrollTo(0, JX.$V(JX.$(config.target)).y - 100);
+    JX.DOM.scrollTo(0, JX.$V(JX.$(config.target)).y - 100);
   }, 0);
 
 });

webroot/rsrc/js/application/releeph/releeph-request-state-change.js

@@ -22,7 +22,7 @@ JX.behavior('releeph-request-state-change', function() {
 
     if (keynav_cursor < 0) {
       keynav_cursor = -1;
-      window.scrollTo(0);
+      JX.DOM.scrollToPosition(0, 0);
       keynavMarkup();
       return;
     }

webroot/rsrc/js/core/KeyboardShortcutManager.js

@@ -66,7 +66,9 @@ JX.install('KeyboardShortcutManager', {
      * Scroll an element into view.
      */
     scrollTo : function(node) {
-      window.scrollTo(0, JX.$V(node).y - 60);
+      var scroll_distance = JX.Vector.getAggregateScrollForNode(node);
+      var node_position = JX.$V(node);
+      JX.DOM.scrollToPosition(0, node_position.y + scroll_distance.y - 60);
     },
 
     /**
@@ -91,8 +93,10 @@ JX.install('KeyboardShortcutManager', {
 
       // Outset the reticle some pixels away from the element, so there's some
       // space between the focused element and the outline.
-      var p  = JX.Vector.getPos(node);
-      p.add(-4, -4).setPos(r);
+      var p = JX.Vector.getPos(node);
+      var s = JX.Vector.getAggregateScrollForNode(node);
+
+      p.add(s).add(-4, -4).setPos(r);
       // Compute the size we need to extend to the full extent of the focused
       // nodes.
       JX.Vector.getPos(extended_node)
@@ -100,7 +104,7 @@ JX.install('KeyboardShortcutManager', {
         .add(JX.Vector.getDim(extended_node))
         .add(8, 8)
         .setDim(r);
-      document.body.appendChild(r);
+      JX.DOM.getContentFrame().appendChild(r);
 
       this._focusReticle = r;
     },

webroot/rsrc/js/core/behavior-scrollbar.js

@@ -5,5 +5,8 @@
  */
 
 JX.behavior('scrollbar', function(config) {
-  new JX.Scrollbar(JX.$(config.nodeID));
+  var bar = new JX.Scrollbar(JX.$(config.nodeID));
+  if (config.isMainContent) {
+    bar.setAsScrollFrame();
+  }
 });

webroot/rsrc/js/core/behavior-watch-anchor.js

@@ -39,7 +39,7 @@ JX.behavior('phabricator-watch-anchor', function() {
       var n = 50;
       var try_anchor_again = function () {
         try {
-          window.scrollTo(0, JX.$V(JX.$(anchor)).y - 60);
+          JX.DOM.scrollToPosition(0, JX.$V(JX.$(anchor)).y - 60);
           defer_highlight();
         } catch (e) {
           if (n--) {