From 49a20bde8e6b735203eabad154cadbf2e384807f Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 2 Dec 2016 10:52:14 -0800
Subject: [PATCH] Prevent double lightbox in lightbox comments

Summary: Ref T3612, prevents lightbox from spawning from inside a lightbox.

Test Plan: Click on file lightbox, leave file comment, click file comment, get take to file page instead of another lightbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16978
---
 resources/celerity/map.php                    | 22 +++++++++----------
 src/view/page/PhabricatorStandardPageView.php |  1 +
 .../js/core/behavior-lightbox-attachments.js  |  7 ++++--
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 328e97b856..5cbe47cfbb 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -10,7 +10,7 @@ return array(
     'conpherence.pkg.css' => '0b64e988',
     'conpherence.pkg.js' => '6249a1cf',
     'core.pkg.css' => '27fd330b',
-    'core.pkg.js' => '14fcb9e4',
+    'core.pkg.js' => 'e4260032',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => 'a4ba74b5',
     'differential.pkg.js' => '634399e9',
@@ -505,7 +505,7 @@ return array(
     'rsrc/js/core/behavior-hovercard.js' => 'bcaccd64',
     'rsrc/js/core/behavior-keyboard-pager.js' => 'a8da01f0',
     'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0',
-    'rsrc/js/core/behavior-lightbox-attachments.js' => 'e089b29b',
+    'rsrc/js/core/behavior-lightbox-attachments.js' => 'ddcd41cf',
     'rsrc/js/core/behavior-line-linker.js' => '1499a8cb',
     'rsrc/js/core/behavior-more.js' => 'a80d0378',
     'rsrc/js/core/behavior-object-selector.js' => 'e0ec7f2f',
@@ -651,7 +651,7 @@ return array(
     'javelin-behavior-history-install' => '7ee2b591',
     'javelin-behavior-icon-composer' => '8499b6ab',
     'javelin-behavior-launch-icon-composer' => '48086888',
-    'javelin-behavior-lightbox-attachments' => 'e089b29b',
+    'javelin-behavior-lightbox-attachments' => 'ddcd41cf',
     'javelin-behavior-line-chart' => 'e4232876',
     'javelin-behavior-load-blame' => '42126667',
     'javelin-behavior-maniphest-batch-editor' => '782ab6e7',
@@ -2050,14 +2050,7 @@ return array(
       'javelin-util',
       'phabricator-shaped-request',
     ),
-    'de2e896f' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-typeahead',
-      'javelin-typeahead-ondemand-source',
-      'javelin-dom',
-    ),
-    'e089b29b' => array(
+    'ddcd41cf' => array(
       'javelin-behavior',
       'javelin-stratcom',
       'javelin-dom',
@@ -2066,6 +2059,13 @@ return array(
       'phuix-icon-view',
       'phabricator-busy',
     ),
+    'de2e896f' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-typeahead',
+      'javelin-typeahead-ondemand-source',
+      'javelin-dom',
+    ),
     'e0ec7f2f' => array(
       'javelin-behavior',
       'javelin-dom',
diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php
index b4208aac73..bd2b448be5 100644
--- a/src/view/page/PhabricatorStandardPageView.php
+++ b/src/view/page/PhabricatorStandardPageView.php
@@ -512,6 +512,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
       'div',
       array(
         'class' => implode(' ', $classes),
+        'id' => 'main-page-frame',
       ),
       array(
         $main_page,
diff --git a/webroot/rsrc/js/core/behavior-lightbox-attachments.js b/webroot/rsrc/js/core/behavior-lightbox-attachments.js
index 55a1e1f59c..930bcb2413 100644
--- a/webroot/rsrc/js/core/behavior-lightbox-attachments.js
+++ b/webroot/rsrc/js/core/behavior-lightbox-attachments.js
@@ -50,7 +50,8 @@ JX.behavior('lightbox-attachments', function (config) {
 
     e.kill();
 
-    var links = JX.DOM.scry(document, 'a', 'lightboxable');
+    var mainFrame = JX.$('main-page-frame');
+    var links = JX.DOM.scry(mainFrame, 'a', 'lightboxable');
     var phids = {};
     var data;
     for (var i = 0; i < links.length; i++) {
@@ -317,7 +318,9 @@ JX.behavior('lightbox-attachments', function (config) {
     el.click();
   }
 
-  JX.Stratcom.listen(
+  // Only look for lightboxable inside the main page, not other lightboxes.
+  JX.DOM.listen(
+    JX.$('main-page-frame'),
     'click',
     ['lightboxable'],
     loadLightBox);