From dbde4b9ff2813e44bb4092b526123fc0154cd8be Mon Sep 17 00:00:00 2001
From: Alan Huang <alanh@fb.com>
Date: Thu, 9 Aug 2012 17:53:05 -0700
Subject: [PATCH] Fix issues with Differential file toggling

Summary:
See D2714#15.

 # Give anchors an `id` equal to their `name` so JX.$ can find them.
 # Listen for `click` s on `<a>` s instead of `hashchange` s to make
   toggling a bit more correct and consistent.
 # Add a placeholder menu item when the file is unloaded. A previous
   patch by @jungejason had left the item blank in that case.
 # In fixing (1) I found another exception when clicking on ToC links.
   Since those links are `differential-load`, they try to load the file
   before jumping to it. However, loading destroys the node it's looking
   for, so if you jump to an already-loaded file JX.$ complains at you.

Test Plan: Make a giant diff. Click on links and try toggling files.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: vrana, aran, Korvin

Maniphest Tasks: T370

Differential Revision: https://secure.phabricator.com/D3185
---
 .../differential/behavior-dropdown-menus.js   |  3 ++
 .../differential/behavior-toggle-files.js     | 45 ++++++++++++++-----
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js
index 90357c6252..a49293f343 100644
--- a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js
+++ b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js
@@ -101,6 +101,8 @@ JX.behavior('differential-dropdown-menus', function(config) {
           reveal_item.setName('Entire File Shown');
         }
 
+        visible_item.setDisabled(true);
+        visible_item.setName("Can't Toggle Unloaded File");
         var diffs = JX.DOM.scry(JX.$(data.containerID),
                                'table', 'differential-diff');
         if (diffs.length > 1) {
@@ -109,6 +111,7 @@ JX.behavior('differential-dropdown-menus', function(config) {
             data.containerID+'."');
         } else if (diffs.length == 1) {
           diff = diffs[0];
+          visible_item.setDisabled(false);
           if (JX.Stratcom.getData(diff).hidden) {
             visible_item.setName('Expand File');
           } else {
diff --git a/webroot/rsrc/js/application/differential/behavior-toggle-files.js b/webroot/rsrc/js/application/differential/behavior-toggle-files.js
index 4d623b35d4..4f01c32055 100644
--- a/webroot/rsrc/js/application/differential/behavior-toggle-files.js
+++ b/webroot/rsrc/js/application/differential/behavior-toggle-files.js
@@ -33,11 +33,15 @@ JX.behavior('differential-toggle-files', function(config) {
     function(e) {
       var elt = e.getData().element;
       while (elt !== document.body) {
-        if (JX.Stratcom.hasSigil(elt, 'differential-diff') &&
-            JX.Stratcom.getData(elt).hidden) {
-          JX.Stratcom.invoke('differential-toggle-file', null, {
-            diff: [ elt ],
-          });
+        if (JX.Stratcom.hasSigil(elt, 'differential-changeset')) {
+          var diffs = JX.DOM.scry(elt, 'table', 'differential-diff');
+          for (var i = 0; i < diffs.length; ++i) {
+            if (JX.Stratcom.getData(diffs[i]).hidden) {
+              JX.Stratcom.invoke('differential-toggle-file', null, {
+                diff: [ diffs[i] ],
+              });
+            }
+          }
           return;
         }
         elt = elt.parentNode;
@@ -45,18 +49,37 @@ JX.behavior('differential-toggle-files', function(config) {
     });
 
   JX.Stratcom.listen(
-    'hashchange',
-    null,
+    'click',
+    'tag:a',
     function(e) {
-      var id = window.location.hash;
-      if (!id.match(/^#/)) {
+      var link = e.getNode('tag:a');
+      var id = link.getAttribute('href');
+      if (!id.match(/^#.+/)) {
         return;
       }
+      // The target may have either a matching name or a matching id.
+      var target;
+      try {
+        target = JX.$(id.substr(1));
+      } catch(err) {
+        var named = document.getElementsByName(id.substr(1));
+        var matches = [];
+        for (var i = 0; i < named.length; ++i) {
+          if (named[i].tagName.toLowerCase() == 'a') {
+            matches.push(named[i]);
+          }
+        }
+        if (matches.length == 1) {
+          target = matches[0];
+        } else {
+          return;
+        }
+      }
       JX.Stratcom.invoke('differential-toggle-file-request', null, {
-        element: JX.$(id.substr(1)),
+        element: target,
       });
       // This event is processed after the hash has changed, so it doesn't
       // automatically jump there like we want.
-      JX.DOM.scrollTo(JX.$(id.substr(1)));
+      JX.DOM.scrollTo(target);
     });
 });