From c3c55d82ae9dcf6d4dae97e292103dda8bff72e2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 17 Apr 2020 15:39:47 -0700 Subject: [PATCH] Make "renderer", "engine", and "encoding" sticky across reloads in Differential and Diffusion Summary: Ref T13455. Update the other "view state" properties to work like "highlight" now works. Some complexity here arises from these concerns: - In "View Standalone", we render the changeset inline. This is useful for debugging/development, and desirable to retain. - In all other cases, we render the changeset with AJAX. So the client needs to be able to learn about the "state" properties of the changeset on two different flows. Prior to this change, each pathway had a fair amount of unique code. Then, some bookkeeping issues: - At inital rendering time, we may not know which renderer will be selected: it may be based on the client viewport dimensions. - Prior to this change, the client didn't separate "value of the property for the changeset as rendered" and "desired value of the property". Test Plan: - Viewed changes in Differential, Diffusion, and in standalone mode. - Toggled renderer, character sets, and document engine (this one isn't terribly useful). Reloaded, saw them stick. - Started typing a comment, cancelled it, hit the undo UI. Maniphest Tasks: T13455 Differential Revision: https://secure.phabricator.com/D21138 --- resources/celerity/map.php | 50 ++++---- .../DifferentialChangesetViewController.php | 11 +- .../parser/DifferentialChangesetParser.php | 117 ++++++++++-------- .../view/DifferentialChangesetDetailView.php | 56 ++++----- .../view/DifferentialChangesetListView.php | 13 +- .../controller/DiffusionDiffController.php | 6 +- .../diff/PhabricatorChangesetResponse.php | 28 +++-- .../PhabricatorChangesetViewState.php | 40 ++++++ .../PhabricatorChangesetViewStateEngine.php | 34 ++++- .../rsrc/js/application/diff/DiffChangeset.js | 95 +++++++------- .../js/application/diff/DiffChangesetList.js | 20 ++- .../rsrc/js/application/diff/DiffInline.js | 2 +- 12 files changed, 268 insertions(+), 204 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 21e0ca0287..33d3bc03cc 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,7 +12,7 @@ return array( 'core.pkg.css' => '86f155f9', 'core.pkg.js' => '705aec2c', 'differential.pkg.css' => '607c84be', - 'differential.pkg.js' => 'd73a942b', + 'differential.pkg.js' => '99e2cb01', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -377,9 +377,9 @@ 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' => '7ccc4153', - 'rsrc/js/application/diff/DiffChangesetList.js' => '2e636e0a', - 'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94', + 'rsrc/js/application/diff/DiffChangeset.js' => '5a4e4a3b', + 'rsrc/js/application/diff/DiffChangesetList.js' => '4769cfe7', + 'rsrc/js/application/diff/DiffInline.js' => '16e97ebc', 'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', 'rsrc/js/application/differential/behavior-populate.js' => 'dfa1d313', @@ -774,9 +774,9 @@ return array( 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => '7ccc4153', - 'phabricator-diff-changeset-list' => '2e636e0a', - 'phabricator-diff-inline' => 'a4a14a94', + 'phabricator-diff-changeset' => '5a4e4a3b', + 'phabricator-diff-changeset-list' => '4769cfe7', + 'phabricator-diff-inline' => '16e97ebc', 'phabricator-drag-and-drop-file-upload' => '4370900d', 'phabricator-draggable-list' => '0169e425', 'phabricator-fatal-config-template-css' => '20babf50', @@ -1029,6 +1029,9 @@ return array( 'javelin-stratcom', 'javelin-util', ), + '16e97ebc' => array( + 'javelin-dom', + ), '1b6acc2a' => array( 'javelin-magical-init', 'javelin-util', @@ -1169,10 +1172,6 @@ return array( 'javelin-util', 'javelin-stratcom', ), - '2e636e0a' => array( - 'javelin-install', - 'phuix-button-view', - ), '2f1db1ed' => array( 'javelin-util', ), @@ -1308,6 +1307,10 @@ return array( 'javelin-util', 'phabricator-busy', ), + '4769cfe7' => array( + 'javelin-install', + 'phuix-button-view', + ), '47a0728b' => array( 'javelin-behavior', 'javelin-dom', @@ -1441,6 +1444,17 @@ return array( 'javelin-util', 'javelin-magical-init', ), + '5a4e4a3b' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '5a6f6a06' => array( 'javelin-behavior', 'javelin-quicksand', @@ -1612,17 +1626,6 @@ return array( 'javelin-install', 'javelin-dom', ), - '7ccc4153' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), '80bff3af' => array( 'javelin-install', 'javelin-typeahead-source', @@ -1828,9 +1831,6 @@ return array( 'javelin-stratcom', 'javelin-vector', ), - 'a4a14a94' => array( - 'javelin-dom', - ), 'a4aa75c4' => array( 'phui-button-css', 'phui-button-simple-css', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index 2dccd60664..80283e9eea 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -205,8 +205,6 @@ final class DifferentialChangesetViewController extends DifferentialController { ->setRightSideCommentMapping($right_source, $right_new) ->setLeftSideCommentMapping($left_source, $left_new); - $parser->readParametersFromRequest($request); - if ($left && $right) { $parser->setOriginals($left, $right); } @@ -274,7 +272,7 @@ final class DifferentialChangesetViewController extends DifferentialController { if ($request->isAjax()) { // NOTE: We must render the changeset before we render coverage // information, since it builds some caches. - $rendered_changeset = $parser->renderChangeset(); + $response = $parser->newChangesetResponse(); $mcov = $parser->renderModifiedCoverage(); @@ -282,10 +280,9 @@ final class DifferentialChangesetViewController extends DifferentialController { 'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov, ); - return id(new PhabricatorChangesetResponse()) - ->setRenderedChangeset($rendered_changeset) - ->setCoverage($coverage_data) - ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); + $response->setCoverage($coverage_data); + + return $response; } $detail = id(new DifferentialChangesetListView()) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 11f2c9822e..e9ae71d63e 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -44,7 +44,6 @@ final class DifferentialChangesetParser extends Phobject { private $highlightErrors; private $disableCache; private $renderer; - private $characterEncoding; private $highlightingDisabled; private $showEditAndReplyLinks = true; private $canMarkDone; @@ -58,7 +57,6 @@ final class DifferentialChangesetParser extends Phobject { private $highlightEngine; private $viewer; - private $documentEngineKey; private $viewState; @@ -95,24 +93,12 @@ final class DifferentialChangesetParser extends Phobject { return $this->viewState; } - public function setCharacterEncoding($character_encoding) { - $this->characterEncoding = $character_encoding; - return $this; - } - - public function getCharacterEncoding() { - return $this->characterEncoding; - } - public function setRenderer(DifferentialChangesetRenderer $renderer) { $this->renderer = $renderer; return $this; } public function getRenderer() { - if (!$this->renderer) { - return new DifferentialChangesetTwoUpRenderer(); - } return $this->renderer; } @@ -161,53 +147,34 @@ final class DifferentialChangesetParser extends Phobject { return $this->viewer; } - public function setDocumentEngineKey($document_engine_key) { - $this->documentEngineKey = $document_engine_key; - return $this; - } + private function newRenderer() { + $viewer = $this->getViewer(); + $viewstate = $this->getViewstate(); - public function getDocumentEngineKey() { - return $this->documentEngineKey; - } + $renderer_key = $viewstate->getRendererKey(); - public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { - $is_unified = $viewer->compareUserSetting( - PhabricatorUnifiedDiffsSetting::SETTINGKEY, - PhabricatorUnifiedDiffsSetting::VALUE_ALWAYS_UNIFIED); + if ($renderer_key === null) { + $is_unified = $viewer->compareUserSetting( + PhabricatorUnifiedDiffsSetting::SETTINGKEY, + PhabricatorUnifiedDiffsSetting::VALUE_ALWAYS_UNIFIED); - if ($is_unified) { - return '1up'; + if ($is_unified) { + $renderer_key = '1up'; + } else { + $renderer_key = $viewstate->getDefaultDeviceRendererKey(); + } } - return null; - } - - public function readParametersFromRequest(AphrontRequest $request) { - $this->setCharacterEncoding($request->getStr('encoding')); - $this->setDocumentEngineKey($request->getStr('engine')); - - $renderer = null; - - // If the viewer prefers unified diffs, always set the renderer to unified. - // Otherwise, we leave it unspecified and the client will choose a - // renderer based on the screen size. - - if ($request->getStr('renderer')) { - $renderer = $request->getStr('renderer'); - } else { - $renderer = self::getDefaultRendererForViewer($request->getViewer()); - } - - switch ($renderer) { + switch ($renderer_key) { case '1up': - $this->setRenderer(new DifferentialChangesetOneUpRenderer()); + $renderer = new DifferentialChangesetOneUpRenderer(); break; default: - $this->setRenderer(new DifferentialChangesetTwoUpRenderer()); + $renderer = new DifferentialChangesetTwoUpRenderer(); break; } - return $this; + return $renderer; } const CACHE_VERSION = 14; @@ -633,7 +600,8 @@ final class DifferentialChangesetParser extends Phobject { $skip_cache = true; } - if ($this->characterEncoding) { + $character_encoding = $viewstate->getCharacterEncoding(); + if ($character_encoding !== null) { $skip_cache = true; } @@ -782,6 +750,12 @@ final class DifferentialChangesetParser extends Phobject { $range_len = null, $mask_force = array()) { + $renderer = $this->getRenderer(); + if (!$renderer) { + $renderer = $this->newRenderer(); + $this->setRenderer($renderer); + } + // "Top level" renders are initial requests for the whole file, versus // requests for a specific range generated by clicking "show more". We // generate property changes and "shield" UI elements only for toplevel @@ -789,14 +763,18 @@ final class DifferentialChangesetParser extends Phobject { $this->isTopLevel = (($range_start === null) && ($range_len === null)); $this->highlightEngine = PhabricatorSyntaxHighlighter::newEngine(); + $viewstate = $this->getViewState(); + $encoding = null; - if ($this->characterEncoding) { + + $character_encoding = $viewstate->getCharacterEncoding(); + if ($character_encoding) { // We are forcing this changeset to be interpreted with a specific // character encoding, so force all the hunks into that encoding and // propagate it to the renderer. - $encoding = $this->characterEncoding; + $encoding = $character_encoding; foreach ($this->changeset->getHunks() as $hunk) { - $hunk->forceEncoding($this->characterEncoding); + $hunk->forceEncoding($character_encoding); } } else { // We're just using the default, so tell the renderer what that is @@ -1794,7 +1772,9 @@ final class DifferentialChangesetParser extends Phobject { } } - $engine_key = $this->getDocumentEngineKey(); + $viewstate = $this->getViewState(); + + $engine_key = $viewstate->getDocumentEngineKey(); if (strlen($engine_key)) { if (isset($shared_engines[$engine_key])) { $document_engine = $shared_engines[$engine_key]; @@ -1895,4 +1875,31 @@ final class DifferentialChangesetParser extends Phobject { return array($old_file, $new_file); } + public function newChangesetResponse() { + // NOTE: This has to happen first because it has side effects. Yuck. + $rendered_changeset = $this->renderChangeset(); + + $renderer = $this->getRenderer(); + $renderer_key = $renderer->getRendererKey(); + + $viewstate = $this->getViewState(); + + $undo_templates = $renderer->renderUndoTemplates(); + foreach ($undo_templates as $key => $undo_template) { + $undo_templates[$key] = hsprintf('%s', $undo_template); + } + + $state = array( + 'undoTemplates' => $undo_templates, + 'rendererKey' => $renderer_key, + 'highlight' => $viewstate->getHighlightLanguage(), + 'characterEncoding' => $viewstate->getCharacterEncoding(), + 'documentEngine' => $viewstate->getDocumentEngineKey(), + ); + + return id(new PhabricatorChangesetResponse()) + ->setRenderedChangeset($rendered_changeset) + ->setChangesetState($state); + } + } diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index 49562744fb..0efb959969 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -11,10 +11,9 @@ final class DifferentialChangesetDetailView extends AphrontView { private $renderURI; private $renderingRef; private $autoload; - private $loaded; - private $renderer; private $repository; private $diff; + private $changesetResponse; public function setAutoload($autoload) { $this->autoload = $autoload; @@ -25,15 +24,6 @@ final class DifferentialChangesetDetailView extends AphrontView { return $this->autoload; } - public function setLoaded($loaded) { - $this->loaded = $loaded; - return $this; - } - - public function getLoaded() { - return $this->loaded; - } - public function setRenderingRef($rendering_ref) { $this->renderingRef = $rendering_ref; return $this; @@ -43,6 +33,15 @@ final class DifferentialChangesetDetailView extends AphrontView { return $this->renderingRef; } + public function setChangesetResponse(PhabricatorChangesetResponse $response) { + $this->changesetResponse = $response; + return $this; + } + + public function getChangesetResponse() { + return $this->changesetResponse; + } + public function setRenderURI($render_uri) { $this->renderURI = $render_uri; return $this; @@ -72,15 +71,6 @@ final class DifferentialChangesetDetailView extends AphrontView { return $this; } - public function setRenderer($renderer) { - $this->renderer = $renderer; - return $this; - } - - public function getRenderer() { - return $this->renderer; - } - public function getID() { if (!$this->id) { $this->id = celerity_generate_unique_node_id(); @@ -139,9 +129,6 @@ final class DifferentialChangesetDetailView extends AphrontView { $icon = id(new PHUIIconView()) ->setIcon($display_icon); - $renderer = DifferentialChangesetHTMLRenderer::getHTMLRendererByKey( - $this->getRenderer()); - $changeset_id = $this->changeset->getID(); $vs_id = $this->getVsChangesetID(); @@ -180,6 +167,17 @@ final class DifferentialChangesetDetailView extends AphrontView { ), $file_part); + $response = $this->getChangesetResponse(); + if ($response) { + $is_loaded = true; + $changeset_markup = $response->getRenderedChangeset(); + $changeset_state = $response->getChangesetState(); + } else { + $is_loaded = false; + $changeset_markup = null; + $changeset_state = null; + } + return javelin_tag( 'div', array( @@ -188,12 +186,8 @@ final class DifferentialChangesetDetailView extends AphrontView { 'left' => $left_id, 'right' => $right_id, 'renderURI' => $this->getRenderURI(), - 'highlight' => null, - 'renderer' => $this->getRenderer(), 'ref' => $this->getRenderingRef(), 'autoload' => $this->getAutoload(), - 'loaded' => $this->getLoaded(), - 'undoTemplates' => hsprintf('%s', $renderer->renderUndoTemplates()), 'displayPath' => hsprintf('%s', $display_parts), 'path' => $display_filename, 'icon' => $display_icon, @@ -201,6 +195,9 @@ final class DifferentialChangesetDetailView extends AphrontView { 'editorURI' => $this->getEditorURI(), 'editorConfigureURI' => $this->getEditorConfigureURI(), + + 'loaded' => $is_loaded, + 'changesetState' => $changeset_state, ), 'class' => $class, 'id' => $id, @@ -225,7 +222,10 @@ final class DifferentialChangesetDetailView extends AphrontView { 'class' => 'changeset-view-content', 'sigil' => 'changeset-view-content', ), - $this->renderChildren()), + array( + $changeset_markup, + $this->renderChildren(), + )), )); } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index de753d291b..1783e7d8ad 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -153,9 +153,6 @@ final class DifferentialChangesetListView extends AphrontView { $changesets = $this->changesets; - $renderer = DifferentialChangesetParser::getDefaultRendererForViewer( - $viewer); - $repository = $this->getRepository(); $diff = $this->getDiff(); @@ -167,7 +164,7 @@ final class DifferentialChangesetListView extends AphrontView { $ref = $this->references[$key]; $detail = id(new DifferentialChangesetDetailView()) - ->setUser($viewer); + ->setViewer($viewer); if ($repository) { $detail->setRepository($repository); @@ -193,11 +190,11 @@ final class DifferentialChangesetListView extends AphrontView { $detail->setRenderingRef($ref); $detail->setRenderURI($this->renderURI); - $detail->setRenderer($renderer); - if ($this->getParser()) { - $detail->appendChild($this->getParser()->renderChangeset()); - $detail->setLoaded(true); + $parser = $this->getParser(); + if ($parser) { + $response = $parser->newChangesetResponse(); + $detail->setChangesetResponse($response); } else { $detail->setAutoload(isset($this->visibleChangesets[$key])); if (isset($this->visibleChangesets[$key])) { diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index 2bb5a989b3..d85c292cb7 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -79,8 +79,6 @@ final class DiffusionDiffController extends DiffusionController { 'action' => 'rendering-ref', ))); - $parser->readParametersFromRequest($request); - $coverage = $drequest->loadCoverage(); if ($coverage) { $parser->setCoverage($coverage); @@ -132,8 +130,6 @@ final class DiffusionDiffController extends DiffusionController { $parser->setRange($range_s, $range_e); $parser->setMask($mask); - return id(new PhabricatorChangesetResponse()) - ->setRenderedChangeset($parser->renderChangeset()) - ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); + return $parser->newChangesetResponse(); } } diff --git a/src/infrastructure/diff/PhabricatorChangesetResponse.php b/src/infrastructure/diff/PhabricatorChangesetResponse.php index 38c604fc06..d9a8e897c7 100644 --- a/src/infrastructure/diff/PhabricatorChangesetResponse.php +++ b/src/infrastructure/diff/PhabricatorChangesetResponse.php @@ -4,20 +4,19 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse { private $renderedChangeset; private $coverage; - private $undoTemplates; + private $changesetState; public function setRenderedChangeset($rendered_changeset) { $this->renderedChangeset = $rendered_changeset; return $this; } - public function setCoverage($coverage) { - $this->coverage = $coverage; - return $this; + public function getRenderedChangeset() { + return $this->renderedChangeset; } - public function setUndoTemplates($undo_templates) { - $this->undoTemplates = $undo_templates; + public function setCoverage($coverage) { + $this->coverage = $coverage; return $this; } @@ -27,18 +26,23 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse { public function reduceProxyResponse() { $content = array( - 'changeset' => $this->renderedChangeset, - ); + 'changeset' => $this->getRenderedChangeset(), + ) + $this->getChangesetState(); if ($this->coverage) { $content['coverage'] = $this->coverage; } - if ($this->undoTemplates) { - $content['undoTemplates'] = $this->undoTemplates; - } - return $this->getProxy()->setContent($content); } + public function setChangesetState(array $state) { + $this->changesetState = $state; + return $this; + } + + public function getChangesetState() { + return $this->changesetState; + } + } diff --git a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewState.php b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewState.php index 3f1e1c1f20..4d29a8fd59 100644 --- a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewState.php +++ b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewState.php @@ -4,6 +4,10 @@ final class PhabricatorChangesetViewState extends Phobject { private $highlightLanguage; + private $characterEncoding; + private $documentEngineKey; + private $rendererKey; + private $defaultDeviceRendererKey; public function setHighlightLanguage($highlight_language) { $this->highlightLanguage = $highlight_language; @@ -14,4 +18,40 @@ final class PhabricatorChangesetViewState return $this->highlightLanguage; } + public function setCharacterEncoding($character_encoding) { + $this->characterEncoding = $character_encoding; + return $this; + } + + public function getCharacterEncoding() { + return $this->characterEncoding; + } + + public function setDocumentEngineKey($document_engine_key) { + $this->documentEngineKey = $document_engine_key; + return $this; + } + + public function getDocumentEngineKey() { + return $this->documentEngineKey; + } + + public function setRendererKey($renderer_key) { + $this->rendererKey = $renderer_key; + return $this; + } + + public function getRendererKey() { + return $this->rendererKey; + } + + public function setDefaultDeviceRendererKey($renderer_key) { + $this->defaultDeviceRendererKey = $renderer_key; + return $this; + } + + public function getDefaultDeviceRendererKey() { + return $this->defaultDeviceRendererKey; + } + } diff --git a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php index d84beaa9be..96e544f560 100644 --- a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php +++ b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php @@ -41,10 +41,25 @@ final class PhabricatorChangesetViewStateEngine $this->setStorage($storage); $highlight = $request->getStr('highlight'); - if ($highlight !== null && strlen($highlight)) { + if ($highlight !== null) { $this->setChangesetProperty('highlight', $highlight); } + $encoding = $request->getStr('encoding'); + if ($encoding !== null) { + $this->setChangesetProperty('encoding', $encoding); + } + + $engine = $request->getStr('engine'); + if ($engine !== null) { + $this->setChangesetProperty('engine', $engine); + } + + $renderer = $request->getStr('renderer'); + if ($renderer !== null) { + $this->setChangesetProperty('renderer', $renderer); + } + $this->saveViewStateStorage(); $state = new PhabricatorChangesetViewState(); @@ -52,6 +67,23 @@ final class PhabricatorChangesetViewStateEngine $highlight_language = $this->getChangesetProperty('highlight'); $state->setHighlightLanguage($highlight_language); + $encoding = $this->getChangesetProperty('encoding'); + $state->setCharacterEncoding($encoding); + + $document_engine = $this->getChangesetProperty('engine'); + $state->setDocumentEngineKey($document_engine); + + $renderer = $this->getChangesetProperty('renderer'); + $state->setRendererKey($renderer); + + // This is the client-selected default renderer based on viewport + // dimensions. + + $device_key = $request->getStr('device'); + if ($device_key !== null && strlen($device_key)) { + $state->setDefaultDeviceRendererKey($device_key); + } + return $state; } diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index 537340b18e..f4c88b6bbf 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -22,10 +22,6 @@ JX.install('DiffChangeset', { this._renderURI = data.renderURI; this._ref = data.ref; - this._renderer = data.renderer; - this._highlight = data.highlight; - this._documentEngine = data.documentEngine; - this._encoding = data.encoding; this._loaded = data.loaded; this._treeNodeID = data.treeNodeID; @@ -39,6 +35,10 @@ JX.install('DiffChangeset', { this._editorConfigureURI = data.editorConfigureURI; this._inlines = []; + + if (data.changesetState) { + this._loadChangesetState(data.changesetState); + } }, members: { @@ -49,10 +49,10 @@ JX.install('DiffChangeset', { _renderURI: null, _ref: null, - _renderer: null, + _rendererKey: null, _highlight: null, _documentEngine: null, - _encoding: null, + _characterEncoding: null, _undoTemplates: null, _leftID: null, @@ -187,11 +187,11 @@ JX.install('DiffChangeset', { * * @return this */ - reload: function() { + reload: function(state) { this._loaded = true; this._sequence++; - var params = this._getViewParameters(); + var params = this._getViewParameters(state); var pht = this.getChangesetList().getTranslations(); var workflow = new JX.Workflow(this._renderURI, params) @@ -321,14 +321,17 @@ JX.install('DiffChangeset', { /** * Get parameters which define the current rendering options. */ - _getViewParameters: function() { - return { + _getViewParameters: function(state) { + var parameters = { ref: this._ref, - renderer: this.getRenderer() || '', - highlight: this._highlight || '', - engine: this._documentEngine || '', - encoding: this._encoding || '' + device: this._getDefaultDeviceRenderer() }; + + if (state) { + JX.copy(parameters, state); + } + + return parameters; }, /** @@ -344,16 +347,11 @@ JX.install('DiffChangeset', { return JX.Router.getInstance().getRoutableByKey(this._getRoutableKey()); }, - setRenderer: function(renderer) { - this._renderer = renderer; - return this; + getRendererKey: function() { + return this._rendererKey; }, - getRenderer: function() { - if (this._renderer !== null) { - return this._renderer; - } - + _getDefaultDeviceRenderer: function() { // NOTE: If you load the page at one device resolution and then resize to // a different one we don't re-render the diffs, because it's a // complicated mess and you could lose inline comments, cursor positions, @@ -365,28 +363,14 @@ JX.install('DiffChangeset', { return this._undoTemplates; }, - setEncoding: function(encoding) { - this._encoding = encoding; - return this; - }, - - getEncoding: function() { - return this._encoding; - }, - - setHighlight: function(highlight) { - this._highlight = highlight; - return this; + getCharacterEncoding: function() { + return this._characterEncoding; }, getHighlight: function() { return this._highlight; }, - setDocumentEngine: function(engine) { - this._documentEngine = engine; - }, - getDocumentEngine: function(engine) { return this._documentEngine; }, @@ -614,19 +598,7 @@ JX.install('DiffChangeset', { _onchangesetresponse: function(response) { // Code shared by autoload and context responses. - if (response.coverage) { - for (var k in response.coverage) { - try { - JX.DOM.replace(JX.$(k), JX.$H(response.coverage[k])); - } catch (ignored) { - // Not terribly important. - } - } - } - - if (response.undoTemplates) { - this._undoTemplates = response.undoTemplates; - } + this._loadChangesetState(response); JX.Stratcom.invoke('differential-inline-comment-refresh'); @@ -635,6 +607,27 @@ JX.install('DiffChangeset', { JX.Stratcom.invoke('resize'); }, + _loadChangesetState: function(state) { + if (state.coverage) { + for (var k in state.coverage) { + try { + JX.DOM.replace(JX.$(k), JX.$H(state.coverage[k])); + } catch (ignored) { + // Not terribly important. + } + } + } + + if (state.undoTemplates) { + this._undoTemplates = state.undoTemplates; + } + + this._rendererKey = state.rendererKey; + this._highlight = state.highlight; + this._characterEncoding = state.characterEncoding; + this._documentEngine = state.documentEngine; + }, + _getContentFrame: function() { return JX.DOM.find(this._node, 'div', 'changeset-view-content'); }, diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index a54848548e..862926bdfa 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -798,15 +798,16 @@ JX.install('DiffChangesetList', { } } - var renderer = changeset.getRenderer(); + var renderer = changeset.getRendererKey(); if (renderer == '1up') { renderer = '2up'; } else { renderer = '1up'; } - changeset.setRenderer(renderer); + changeset.reload({renderer: renderer}); + } else { + changeset.reload(); } - changeset.reload(); e.prevent(); menu.close(); @@ -818,13 +819,12 @@ JX.install('DiffChangesetList', { .setName(pht('Change Text Encoding...')) .setHandler(function(e) { var params = { - encoding: changeset.getEncoding() + encoding: changeset.getCharacterEncoding() }; new JX.Workflow('/services/encoding/', params) .setHandler(function(r) { - changeset.setEncoding(r.encoding); - changeset.reload(); + changeset.reload({encoding: r.encoding}); }) .start(); @@ -843,8 +843,7 @@ JX.install('DiffChangesetList', { new JX.Workflow('/services/highlight/', params) .setHandler(function(r) { - changeset.setHighlight(r.highlight); - changeset.reload(); + changeset.reload({highlight: r.highlight}); }) .start(); @@ -863,8 +862,7 @@ JX.install('DiffChangesetList', { new JX.Workflow('/services/viewas/', params) .setHandler(function(r) { - changeset.setDocumentEngine(r.engine); - changeset.reload(); + changeset.reload({engine: r.engine}); }) .start(); @@ -917,7 +915,7 @@ JX.install('DiffChangesetList', { engine_item.setDisabled(!changeset.isLoaded()); if (changeset.isLoaded()) { - if (changeset.getRenderer() == '2up') { + if (changeset.getRendererKey() == '2up') { up_item .setIcon('fa-list-alt') .setName(pht('View Unified')); diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index fef6b2087a..3cc32a9357 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -461,7 +461,7 @@ JX.install('DiffInline', { op: operation, id: this._id, on_right: ((this.getDisplaySide() == 'right') ? 1 : 0), - renderer: this.getChangeset().getRenderer(), + renderer: this.getChangeset().getRendererKey(), number: this.getLineNumber(), length: this.getLineLength(), is_new: this.isNewFile(),