From 6fa507987d713a7b9aa328d0cfb037433cc97d40 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 10 Mar 2015 15:32:15 -0700 Subject: [PATCH] Generalize URI pattern blacklist for Quicksand Summary: Fixes T7060. Removes some hard-coding. This assumes that "pages with no durable column" and "pages with no Quicksand" are the same, but that's correct today and I can't come up with a use case where they'd be different offhand. Test Plan: - Clicked a revision with column open, got Quicksand navigation. - Clicked into Conpherence with column open, got real navigation. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7060 Differential Revision: https://secure.phabricator.com/D12036 --- resources/celerity/map.php | 42 ++++++++------- .../base/PhabricatorApplication.php | 4 ++ .../PhabricatorConpherenceApplication.php | 6 +++ .../view/ConpherenceLayoutView.php | 4 +- src/view/page/PhabricatorStandardPageView.php | 50 +++++++++++++----- .../rsrc/externals/javelin/lib/Quicksand.js | 52 +++++++++++++++++++ .../behavior-quicksand-blacklist.js | 9 ++++ 7 files changed, 136 insertions(+), 31 deletions(-) create mode 100644 webroot/rsrc/js/application/conpherence/behavior-quicksand-blacklist.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 41e2e1a911..45eb9b6314 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -44,7 +44,7 @@ return array( 'rsrc/css/application/config/config-welcome.css' => '6abd79be', 'rsrc/css/application/config/setup-issue.css' => '22270af2', 'rsrc/css/application/config/unhandled-exception.css' => '37d4f9a2', - 'rsrc/css/application/conpherence/durable-column.css' => '7abcc3f2', + 'rsrc/css/application/conpherence/durable-column.css' => '9207426d', 'rsrc/css/application/conpherence/menu.css' => 'c6ac5299', 'rsrc/css/application/conpherence/message-pane.css' => '5930260a', 'rsrc/css/application/conpherence/notification.css' => '04a6e10a', @@ -202,7 +202,7 @@ return array( 'rsrc/externals/javelin/lib/JSON.js' => '69adf288', 'rsrc/externals/javelin/lib/Leader.js' => '331b1611', 'rsrc/externals/javelin/lib/Mask.js' => '8a41885b', - 'rsrc/externals/javelin/lib/Quicksand.js' => 'f960d43d', + 'rsrc/externals/javelin/lib/Quicksand.js' => '2bb920b6', 'rsrc/externals/javelin/lib/Request.js' => '94b750d2', 'rsrc/externals/javelin/lib/Resource.js' => '44959b73', 'rsrc/externals/javelin/lib/Routable.js' => 'b3e7d692', @@ -353,9 +353,10 @@ return array( 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'efef202b', - 'rsrc/js/application/conpherence/behavior-durable-column.js' => 'd8dab826', + 'rsrc/js/application/conpherence/behavior-durable-column.js' => '1eef9f26', 'rsrc/js/application/conpherence/behavior-menu.js' => 'e476c952', 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', + 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '2c1cd7f5', 'rsrc/js/application/countdown/timer.js' => 'e4cc26b3', 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '469c0d9e', @@ -514,7 +515,7 @@ return array( 'changeset-view-manager' => '88be0133', 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', - 'conpherence-durable-column-view' => '7abcc3f2', + 'conpherence-durable-column-view' => '9207426d', 'conpherence-menu-css' => 'c6ac5299', 'conpherence-message-pane-css' => '5930260a', 'conpherence-notification-css' => '04a6e10a', @@ -585,7 +586,7 @@ return array( 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', 'javelin-behavior-doorkeeper-tag' => 'e5822781', - 'javelin-behavior-durable-column' => 'd8dab826', + 'javelin-behavior-durable-column' => '1eef9f26', 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-fancy-datepicker' => 'c51ae228', 'javelin-behavior-global-drag-and-drop' => '07f199d8', @@ -640,6 +641,7 @@ return array( 'javelin-behavior-ponder-votebox' => '4e9b766b', 'javelin-behavior-project-boards' => '87cb6b51', 'javelin-behavior-project-create' => '065227cc', + 'javelin-behavior-quicksand-blacklist' => '7927a7d3', 'javelin-behavior-refresh-csrf' => '7814b593', 'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf', 'javelin-behavior-releeph-request-state-change' => 'a0b57eb8', @@ -670,7 +672,7 @@ return array( 'javelin-leader' => '331b1611', 'javelin-magical-init' => '2bd3c675', 'javelin-mask' => '8a41885b', - 'javelin-quicksand' => 'f960d43d', + 'javelin-quicksand' => '2bb920b6', 'javelin-reactor' => '2b8de964', 'javelin-reactor-dom' => 'c90a04fc', 'javelin-reactor-node-calmer' => '76f4ebed', @@ -954,6 +956,15 @@ return array( 'javelin-dom', 'javelin-reactor-dom', ), + '1eef9f26' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-scrollbar', + 'javelin-quicksand', + 'phabricator-keyboard-shortcut', + 'conpherence-thread-manager', + ), '1feea462' => array( 'javelin-install', 'javelin-dom', @@ -1012,6 +1023,9 @@ return array( 'javelin-install', 'javelin-util', ), + '2bb920b6' => array( + 'javelin-install', + ), '2be71d56' => array( 'javelin-install', 'javelin-util', @@ -1360,6 +1374,10 @@ return array( 'javelin-util', 'phabricator-busy', ), + '7927a7d3' => array( + 'javelin-behavior', + 'javelin-quicksand', + ), '7a68dda3' => array( 'owners-path-editor', 'javelin-behavior', @@ -1786,15 +1804,6 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), - 'd8dab826' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-scrollbar', - 'javelin-quicksand', - 'phabricator-keyboard-shortcut', - 'conpherence-thread-manager', - ), 'dbbf48b6' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1989,9 +1998,6 @@ return array( 'javelin-stratcom', 'javelin-uri', ), - 'f960d43d' => array( - 'javelin-install', - ), 'fa0f4fc2' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index bcfcce952d..5f550cc11c 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -185,6 +185,10 @@ abstract class PhabricatorApplication implements PhabricatorPolicyInterface { return array(); } + public function getQuicksandURIPatternBlacklist() { + return array(); + } + /* -( URI Routing )-------------------------------------------------------- */ diff --git a/src/applications/conpherence/application/PhabricatorConpherenceApplication.php b/src/applications/conpherence/application/PhabricatorConpherenceApplication.php index ea2ccef1de..3a16c1257f 100644 --- a/src/applications/conpherence/application/PhabricatorConpherenceApplication.php +++ b/src/applications/conpherence/application/PhabricatorConpherenceApplication.php @@ -56,4 +56,10 @@ final class PhabricatorConpherenceApplication extends PhabricatorApplication { return $items; } + public function getQuicksandURIPatternBlacklist() { + return array( + '/conpherence/.*', + ); + } + } diff --git a/src/applications/conpherence/view/ConpherenceLayoutView.php b/src/applications/conpherence/view/ConpherenceLayoutView.php index 92acf371fe..b67e1e1e43 100644 --- a/src/applications/conpherence/view/ConpherenceLayoutView.php +++ b/src/applications/conpherence/view/ConpherenceLayoutView.php @@ -67,9 +67,11 @@ final class ConpherenceLayoutView extends AphrontView { $selected_id = null; $selected_thread_id = null; + $selected_thread_phid = null; if ($this->thread) { $selected_id = $this->thread->getPHID().'-nav-item'; $selected_thread_id = $this->thread->getID(); + $selected_thread_phid = $this->thread->getPHID(); } $this->initBehavior('conpherence-menu', array( @@ -77,7 +79,7 @@ final class ConpherenceLayoutView extends AphrontView { 'layoutID' => $layout_id, 'selectedID' => $selected_id, 'selectedThreadID' => $selected_thread_id, - 'selectedThreadPHID' => $this->thread->getPHID(), + 'selectedThreadPHID' => $selected_thread_phid, 'latestTransactionID' => $this->latestTransactionID, 'role' => $this->role, 'hasThreadList' => (bool)$this->threadView, diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index ca33060986..d2925793f3 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -81,21 +81,31 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { public function getShowDurableColumn() { $request = $this->getRequest(); - if ($request) { - if (strncmp( - $request->getRequestURI()->getPath(), - '/conpherence', - strlen('/conpherence')) === 0) { + if (!$request) { + return false; + } + + $viewer = $request->getUser(); + if (!$viewer->isLoggedIn()) { + return false; + } + + $conpherence_installed = PhabricatorApplication::isClassInstalledForViewer( + 'PhabricatorConpherenceApplication', + $viewer); + if (!$conpherence_installed) { + return false; + } + + $patterns = $this->getQuicksandURIPatternBlacklist(); + $path = $request->getRequestURI()->getPath(); + foreach ($patterns as $pattern) { + if (preg_match('(^'.$pattern.'$)', $path)) { return false; } - $viewer = $request->getUser(); - if ($viewer->isLoggedIn()) { - return PhabricatorApplication::isClassInstalledForViewer( - 'PhabricatorConpherenceApplication', - $viewer); - } } - return false; + + return true; } public function getTitle() { @@ -414,6 +424,10 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { array()); } + Javelin::initBehavior('quicksand-blacklist', array( + 'patterns' => $this->getQuicksandURIPatternBlacklist(), + )); + return phutil_tag( 'div', array( @@ -585,4 +599,16 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { 'content' => hsprintf('%s', $response), ); } + + private function getQuicksandURIPatternBlacklist() { + $applications = PhabricatorApplication::getAllApplications(); + + $blacklist = array(); + foreach ($applications as $application) { + $blacklist[] = $application->getQuicksandURIPatternBlacklist(); + } + + return array_mergev($blacklist); + } + } diff --git a/webroot/rsrc/externals/javelin/lib/Quicksand.js b/webroot/rsrc/externals/javelin/lib/Quicksand.js index 43b2fa0dae..482ea733e5 100644 --- a/webroot/rsrc/externals/javelin/lib/Quicksand.js +++ b/webroot/rsrc/externals/javelin/lib/Quicksand.js @@ -34,6 +34,7 @@ JX.install('Quicksand', { _started: false, _frameNode: null, _contentNode: null, + _uriPatternBlacklist: [], /** * Start Quicksand, accepting a fate of eternal torment. @@ -124,6 +125,11 @@ JX.install('Quicksand', { return; } + if (self._isURIOnBlacklist(uri)) { + // This URI is blacklisted as not navigable via Quicksand. + return; + } + // The fate of this action is sealed. Suck it into the depths. e.kill(); @@ -276,7 +282,53 @@ JX.install('Quicksand', { .setPort(null) .setDomain(null) .toString(); + }, + + + /** + * Set a list of regular expressions which blacklist URIs as not navigable + * via Quicksand. + * + * If a user clicks a link to one of these URIs, a normal page navigation + * event will occur instead of a Quicksand navigation. + * + * @param list List of regular expressions. + * @return self + */ + setURIPatternBlacklist: function(items) { + var self = JX.Quicksand; + + var list = []; + for (var ii = 0; ii < items.length; ii++) { + list.push(new RegExp('^' + items[ii] + '$')); + } + + self._uriPatternBlacklist = list; + + return self; + }, + + + /** + * Test if a @{class:JX.URI} is on the URI pattern blacklist. + * + * @param JX.URI URI to test. + * @return bool True if the URI is on the blacklist. + */ + _isURIOnBlacklist: function(uri) { + var self = JX.Quicksand; + var list = self._uriPatternBlacklist; + + var path = uri.getPath(); + for (var ii = 0; ii < list.length; ii++) { + if (list[ii].test(path)) { + return true; + } + } + + return false; } + } }); diff --git a/webroot/rsrc/js/application/conpherence/behavior-quicksand-blacklist.js b/webroot/rsrc/js/application/conpherence/behavior-quicksand-blacklist.js new file mode 100644 index 0000000000..48d87d8fe5 --- /dev/null +++ b/webroot/rsrc/js/application/conpherence/behavior-quicksand-blacklist.js @@ -0,0 +1,9 @@ +/** + * @provides javelin-behavior-quicksand-blacklist + * @requires javelin-behavior + * javelin-quicksand + */ + +JX.behavior('quicksand-blacklist', function(config) { + JX.Quicksand.setURIPatternBlacklist(config.patterns); +});