From 0f0e94ca71fe6c3320e6f13e1f856fe95e15ecac Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Oct 2020 09:09:10 -0700 Subject: [PATCH] Use "getInlines()", not "_inlines", to access inlines on client Changeset objects Summary: See PHI1898. An install is reporting an execution/initialization order issue where this code is reachable before `_inlines` is initialized. I can't immediately reproduce it, but using "getInlines()" is preferable anyway and seems likely to fix the problem. Test Plan: Viewed revisions with inlines, added/removed/edited/replied to inlines, didn't find anything broken. Differential Revision: https://secure.phabricator.com/D21475 --- resources/celerity/map.php | 22 +++++++++---------- .../rsrc/js/application/diff/DiffChangeset.js | 19 ++++++++-------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ffa1e95405..4b33b32da5 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => 'adc34883', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => '5c459f92', - 'differential.pkg.js' => '218fda21', + 'differential.pkg.js' => '5080baf4', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => '8ee48a4b', 'maniphest.pkg.css' => '35995d6d', @@ -383,7 +383,7 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', - 'rsrc/js/application/diff/DiffChangeset.js' => '39dcf2c3', + 'rsrc/js/application/diff/DiffChangeset.js' => '3b6e1fde', 'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5', 'rsrc/js/application/diff/DiffInline.js' => '511a1315', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', @@ -784,7 +784,7 @@ return array( 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => '39dcf2c3', + 'phabricator-diff-changeset' => '3b6e1fde', 'phabricator-diff-changeset-list' => 'cc2c5de5', 'phabricator-diff-inline' => '511a1315', 'phabricator-diff-path-view' => '8207abf9', @@ -1229,7 +1229,14 @@ return array( 'trigger-rule', 'trigger-rule-type', ), - '39dcf2c3' => array( + '3ae89b20' => array( + 'phui-workcard-view-css', + ), + '3b4899b0' => array( + 'javelin-behavior', + 'phabricator-prefab', + ), + '3b6e1fde' => array( 'javelin-dom', 'javelin-util', 'javelin-stratcom', @@ -1243,13 +1250,6 @@ return array( 'phuix-button-view', 'javelin-external-editor-link-engine', ), - '3ae89b20' => array( - 'phui-workcard-view-css', - ), - '3b4899b0' => array( - 'javelin-behavior', - 'phabricator-prefab', - ), '3be6ef4f' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index 212d19f5c6..668634cb30 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -723,16 +723,19 @@ JX.install('DiffChangeset', { var data = JX.Stratcom.getData(node); if (!data.inline) { - var inline = new JX.DiffInline() - .setChangeset(this) - .bindToRow(node); - - this._inlines.push(inline); + var inline = this._newInlineForRow(node); + this.getInlines().push(inline); } return data.inline; }, + _newInlineForRow: function(node) { + return new JX.DiffInline() + .setChangeset(this) + .bindToRow(node); + }, + newInlineForRange: function(origin, target, options) { var list = this.getChangesetList(); @@ -770,7 +773,7 @@ JX.install('DiffChangeset', { .setChangeset(this) .bindToRange(data); - this._inlines.push(inline); + this.getInlines().push(inline); inline.create(); @@ -855,9 +858,7 @@ JX.install('DiffChangeset', { continue; } - // As a side effect, this builds any missing inline objects and adds - // them to this Changeset's list of inlines. - this.getInlineForRow(row); + this._inlines.push(this._newInlineForRow(row)); } },