From dc7e40ff3fc7c1e882ebf0b6c6002c0dc33f9d6e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 13 Mar 2018 16:26:01 -0700 Subject: [PATCH] Fix the DarkConsole inline error log stack trace expansion behavior for Content-Security-Policy Summary: See PHI451. Ref T13102. DarkConsole uses an ancient inline "onclick" handler to expand the stack traces for errors. The new Content-Security-Policy prevents this from functioning. Replace this with a more modern behavior-driven action instead. Test Plan: - Clicked some errors in DarkConsole, saw stack traces appear. - Grepped for `onclick` and `jsprintf()` to see if I could find any more of these, but came up empty. Maniphest Tasks: T13102 Differential Revision: https://secure.phabricator.com/D19218 --- resources/celerity/map.php | 34 +++++++------------ resources/celerity/packages.php | 4 --- .../plugin/DarkConsoleErrorLogPlugin.php | 7 ++-- src/view/page/PhabricatorStandardPageView.php | 3 -- webroot/rsrc/js/core/behavior-error-log.js | 19 ----------- .../core/darkconsole/behavior-dark-console.js | 20 +++++++++++ 6 files changed, 37 insertions(+), 50 deletions(-) delete mode 100644 webroot/rsrc/js/core/behavior-error-log.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 769e8dfabe..15388ab9a2 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,7 +11,6 @@ return array( 'conpherence.pkg.js' => '15191c65', 'core.pkg.css' => 'c218ed53', 'core.pkg.js' => '0fabde4f', - 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '113e692c', 'differential.pkg.js' => 'f6d809c0', 'diffusion.pkg.css' => 'a2d17c7d', @@ -462,7 +461,6 @@ return array( 'rsrc/js/core/behavior-detect-timezone.js' => '4c193c96', 'rsrc/js/core/behavior-device.js' => 'a3714c76', 'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '484a6e22', - 'rsrc/js/core/behavior-error-log.js' => '6882e80a', 'rsrc/js/core/behavior-fancy-datepicker.js' => 'ecf4e799', 'rsrc/js/core/behavior-file-tree.js' => '88236f00', 'rsrc/js/core/behavior-form.js' => '5c54cbf3', @@ -501,7 +499,7 @@ return array( 'rsrc/js/core/behavior-workflow.js' => '0a3f3021', 'rsrc/js/core/darkconsole/DarkLog.js' => 'c8e1ffe3', 'rsrc/js/core/darkconsole/DarkMessage.js' => 'c48cccdd', - 'rsrc/js/core/darkconsole/behavior-dark-console.js' => '17bb8539', + 'rsrc/js/core/darkconsole/behavior-dark-console.js' => '66888767', 'rsrc/js/core/phtize.js' => 'd254d646', 'rsrc/js/phui/behavior-phui-dropdown-menu.js' => 'b95d6f7d', 'rsrc/js/phui/behavior-phui-file-upload.js' => 'b003d4fb', @@ -588,7 +586,7 @@ return array( 'javelin-behavior-conpherence-pontificate' => '55616e04', 'javelin-behavior-conpherence-search' => '9bbf3762', 'javelin-behavior-countdown-timer' => 'e4cc26b3', - 'javelin-behavior-dark-console' => '17bb8539', + 'javelin-behavior-dark-console' => '66888767', 'javelin-behavior-dashboard-async-panel' => '469c0d9e', 'javelin-behavior-dashboard-move-panels' => '408bf173', 'javelin-behavior-dashboard-query-panel-select' => '453c5375', @@ -613,7 +611,6 @@ return array( 'javelin-behavior-durable-column' => '2ae077e1', 'javelin-behavior-editengine-reorder-configs' => 'd7a74243', 'javelin-behavior-editengine-reorder-fields' => 'b59e1e96', - 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-event-all-day' => 'b41537c9', 'javelin-behavior-fancy-datepicker' => 'ecf4e799', 'javelin-behavior-global-drag-and-drop' => '960f6a39', @@ -987,16 +984,6 @@ return array( 'aphront-typeahead-control-css', 'phui-tag-view-css', ), - '17bb8539' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-util', - 'javelin-dom', - 'javelin-request', - 'phabricator-keyboard-shortcut', - 'phabricator-darklog', - 'phabricator-darkmessage', - ), '1802a242' => array( 'phui-theme-css', ), @@ -1416,6 +1403,16 @@ return array( 'javelin-typeahead-ondemand-source', 'javelin-util', ), + 66888767 => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-util', + 'javelin-dom', + 'javelin-request', + 'phabricator-keyboard-shortcut', + 'phabricator-darklog', + 'phabricator-darkmessage', + ), '66a6def1' => array( 'javelin-behavior', 'javelin-dom', @@ -1429,9 +1426,6 @@ return array( 'javelin-dom', 'phabricator-notification', ), - '6882e80a' => array( - 'javelin-dom', - ), '68af71ca' => array( 'javelin-install', 'javelin-dom', @@ -2343,10 +2337,6 @@ return array( 'javelin-behavior-user-menu', 'phabricator-favicon', ), - 'darkconsole.pkg.js' => array( - 'javelin-behavior-dark-console', - 'javelin-behavior-error-log', - ), 'differential.pkg.css' => array( 'differential-core-view-css', 'differential-changeset-view-css', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index abbf1d77e2..3cec508678 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -224,8 +224,4 @@ return array( 'javelin-behavior-maniphest-subpriority-editor', 'javelin-behavior-maniphest-list-editor', ), - 'darkconsole.pkg.js' => array( - 'javelin-behavior-dark-console', - 'javelin-behavior-error-log', - ), ); diff --git a/src/applications/console/plugin/DarkConsoleErrorLogPlugin.php b/src/applications/console/plugin/DarkConsoleErrorLogPlugin.php index 0d9604dcba..213b578c75 100644 --- a/src/applications/console/plugin/DarkConsoleErrorLogPlugin.php +++ b/src/applications/console/plugin/DarkConsoleErrorLogPlugin.php @@ -39,10 +39,13 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin { $file = $row['file']; $line = $row['line']; - $tag = phutil_tag( + $tag = javelin_tag( 'a', array( - 'onclick' => jsprintf('show_details(%d)', $index), + 'sigil' => 'darkconsole-expand', + 'meta' => array( + 'expandID' => 'row-details-'.$index, + ), ), $row['str'].' at ['.basename($file).':'.$line.']'); $rows[] = array($tag); diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index aca98b7205..99143add5f 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -330,9 +330,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView Javelin::initBehavior( 'dark-console', $this->getConsoleConfig()); - - // Change this to initBehavior when there is some behavior to initialize - require_celerity_resource('javelin-behavior-error-log'); } if ($user) { diff --git a/webroot/rsrc/js/core/behavior-error-log.js b/webroot/rsrc/js/core/behavior-error-log.js deleted file mode 100644 index 24f3f61141..0000000000 --- a/webroot/rsrc/js/core/behavior-error-log.js +++ /dev/null @@ -1,19 +0,0 @@ -/** - * @provides javelin-behavior-error-log - * @requires javelin-dom - */ - -/* exported show_details */ - -var current_details = null; - -function show_details(row) { - var node = JX.$('row-details-' + row); - - if (current_details !== null) { - JX.$('row-details-' + current_details).style.display = 'none'; - } - - node.style.display = 'block'; - current_details = row; -} diff --git a/webroot/rsrc/js/core/darkconsole/behavior-dark-console.js b/webroot/rsrc/js/core/darkconsole/behavior-dark-console.js index 13949432ff..a9f33d4594 100644 --- a/webroot/rsrc/js/core/darkconsole/behavior-dark-console.js +++ b/webroot/rsrc/js/core/darkconsole/behavior-dark-console.js @@ -404,4 +404,24 @@ JX.behavior('dark-console', function(config, statics) { } + if (!statics.expand) { + statics.expand = true; + + var current_details = null; + JX.Stratcom.listen('click', 'darkconsole-expand', function(e) { + e.kill(); + + if (current_details) { + current_details.style.display = 'none'; + current_details = null; + } + + var id = e.getNodeData('darkconsole-expand').expandID; + var node = JX.$(id); + + node.style.display = 'block'; + current_details = node; + }); + } + });