Improve minor client behaviors for document rendering

Summary:
Ref T13105. This adds various small client-side improvements to document rendering.

  - In the menu, show which renderer is in use.
  - Make linking to lines work.
  - Make URIs persist information about which rendering engine is in use.
  - Improve the UI feedback for transitions between document types.
  - Load slower documents asynchronously by default.
  - Discard irrelevant requests if you spam the view menu.

Test Plan: Loaded files, linked to lines, swapped between modes, copy/pasted URLs.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19256

resources/celerity/map.php

@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => 'e68cf1fa',
     'conpherence.pkg.js' => '15191c65',
-    'core.pkg.css' => 'da541195',
+    'core.pkg.css' => '3fd3b7b8',
     'core.pkg.js' => 'b9b4a943',
     'differential.pkg.css' => '113e692c',
     'differential.pkg.js' => 'f6d809c0',
@@ -168,7 +168,7 @@ return array(
     'rsrc/css/phui/phui-object-box.css' => '9cff003c',
     'rsrc/css/phui/phui-pager.css' => 'edcbc226',
     'rsrc/css/phui/phui-pinboard-view.css' => '2495140e',
-    'rsrc/css/phui/phui-property-list-view.css' => '54c071ed',
+    'rsrc/css/phui/phui-property-list-view.css' => 'de4754d8',
     'rsrc/css/phui/phui-remarkup-preview.css' => '54a34863',
     'rsrc/css/phui/phui-segment-bar-view.css' => 'b1d1b892',
     'rsrc/css/phui/phui-spacing.css' => '042804d6',
@@ -392,7 +392,7 @@ return array(
     'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'f01586dc',
     'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => '1db13e70',
     'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef',
-    'rsrc/js/application/files/behavior-document-engine.js' => '396ef112',
+    'rsrc/js/application/files/behavior-document-engine.js' => 'd3f8623c',
     'rsrc/js/application/files/behavior-icon-composer.js' => '8499b6ab',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888',
     'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => '191b4909',
@@ -473,7 +473,7 @@ return array(
     'rsrc/js/core/behavior-keyboard-pager.js' => 'a8da01f0',
     'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0',
     'rsrc/js/core/behavior-lightbox-attachments.js' => '6b31879a',
-    'rsrc/js/core/behavior-line-linker.js' => 'a9b946f8',
+    'rsrc/js/core/behavior-line-linker.js' => '13e39479',
     'rsrc/js/core/behavior-more.js' => 'a80d0378',
     'rsrc/js/core/behavior-object-selector.js' => '77c1f0b0',
     'rsrc/js/core/behavior-oncopy.js' => '2926fff2',
@@ -607,7 +607,7 @@ return array(
     'javelin-behavior-diffusion-jump-to' => '73d09eef',
     'javelin-behavior-diffusion-locate-file' => '6d3e1947',
     'javelin-behavior-diffusion-pull-lastmodified' => 'f01586dc',
-    'javelin-behavior-document-engine' => '396ef112',
+    'javelin-behavior-document-engine' => 'd3f8623c',
     'javelin-behavior-doorkeeper-tag' => '1db13e70',
     'javelin-behavior-drydock-live-operation-status' => '901935ef',
     'javelin-behavior-durable-column' => '2ae077e1',
@@ -638,7 +638,7 @@ return array(
     'javelin-behavior-phabricator-gesture-example' => '558829c2',
     'javelin-behavior-phabricator-keyboard-pager' => 'a8da01f0',
     'javelin-behavior-phabricator-keyboard-shortcuts' => '01fca1f0',
-    'javelin-behavior-phabricator-line-linker' => 'a9b946f8',
+    'javelin-behavior-phabricator-line-linker' => '13e39479',
     'javelin-behavior-phabricator-nav' => '836f966d',
     'javelin-behavior-phabricator-notification-example' => '8ce821c5',
     'javelin-behavior-phabricator-object-selector' => '77c1f0b0',
@@ -850,7 +850,7 @@ return array(
     'phui-oi-simple-ui-css' => 'a8beebea',
     'phui-pager-css' => 'edcbc226',
     'phui-pinboard-view-css' => '2495140e',
-    'phui-property-list-view-css' => '54c071ed',
+    'phui-property-list-view-css' => 'de4754d8',
     'phui-remarkup-preview-css' => '54a34863',
     'phui-segment-bar-view-css' => 'b1d1b892',
     'phui-spacing-css' => '042804d6',
@@ -964,6 +964,12 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
+    '13e39479' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-history',
+    ),
     '15d5ff71' => array(
       'aphront-typeahead-control-css',
       'phui-tag-view-css',
@@ -1103,11 +1109,6 @@ return array(
       'javelin-dom',
       'javelin-vector',
     ),
-    '396ef112' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-stratcom',
-    ),
     '3ab51e2c' => array(
       'javelin-behavior',
       'javelin-behavior-device',
@@ -1751,12 +1752,6 @@ return array(
       'javelin-uri',
       'phabricator-keyboard-shortcut',
     ),
-    'a9b946f8' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-history',
-    ),
     'a9f88de2' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2007,6 +2002,11 @@ return array(
     'd254d646' => array(
       'javelin-util',
     ),
+    'd3f8623c' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-stratcom',
+    ),
     'd4505101' => array(
       'javelin-stratcom',
       'javelin-install',

src/__phutil_library_map__.php

@@ -3015,7 +3015,6 @@ phutil_register_library_map(array(
     'PhabricatorFileImageMacro' => 'applications/macro/storage/PhabricatorFileImageMacro.php',
     'PhabricatorFileImageProxyController' => 'applications/files/controller/PhabricatorFileImageProxyController.php',
     'PhabricatorFileImageTransform' => 'applications/files/transform/PhabricatorFileImageTransform.php',
-    'PhabricatorFileInfoController' => 'applications/files/controller/PhabricatorFileInfoController.php',
     'PhabricatorFileIntegrityException' => 'applications/files/exception/PhabricatorFileIntegrityException.php',
     'PhabricatorFileLightboxController' => 'applications/files/controller/PhabricatorFileLightboxController.php',
     'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php',
@@ -3051,6 +3050,7 @@ phutil_register_library_map(array(
     'PhabricatorFileUploadException' => 'applications/files/exception/PhabricatorFileUploadException.php',
     'PhabricatorFileUploadSource' => 'applications/files/uploadsource/PhabricatorFileUploadSource.php',
     'PhabricatorFileUploadSourceByteLimitException' => 'applications/files/uploadsource/PhabricatorFileUploadSourceByteLimitException.php',
+    'PhabricatorFileViewController' => 'applications/files/controller/PhabricatorFileViewController.php',
     'PhabricatorFileinfoSetupCheck' => 'applications/config/check/PhabricatorFileinfoSetupCheck.php',
     'PhabricatorFilesApplication' => 'applications/files/application/PhabricatorFilesApplication.php',
     'PhabricatorFilesApplicationStorageEnginePanel' => 'applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php',
@@ -8622,7 +8622,6 @@ phutil_register_library_map(array(
     ),
     'PhabricatorFileImageProxyController' => 'PhabricatorFileController',
     'PhabricatorFileImageTransform' => 'PhabricatorFileTransform',
-    'PhabricatorFileInfoController' => 'PhabricatorFileController',
     'PhabricatorFileIntegrityException' => 'Exception',
     'PhabricatorFileLightboxController' => 'PhabricatorFileController',
     'PhabricatorFileLinkView' => 'AphrontTagView',
@@ -8658,6 +8657,7 @@ phutil_register_library_map(array(
     'PhabricatorFileUploadException' => 'Exception',
     'PhabricatorFileUploadSource' => 'Phobject',
     'PhabricatorFileUploadSourceByteLimitException' => 'Exception',
+    'PhabricatorFileViewController' => 'PhabricatorFileController',
     'PhabricatorFileinfoSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorFilesApplication' => 'PhabricatorApplication',
     'PhabricatorFilesApplicationStorageEnginePanel' => 'PhabricatorApplicationConfigurationPanel',

src/applications/files/application/PhabricatorFilesApplication.php

@@ -69,9 +69,15 @@ final class PhabricatorFilesApplication extends PhabricatorApplication {
 
   public function getRoutes() {
     return array(
-      '/F(?P<id>[1-9]\d*)' => 'PhabricatorFileInfoController',
+      '/F(?P<id>[1-9]\d*)(?:\$(?P<lines>\d+(?:-\d+)?))?'
+        => 'PhabricatorFileViewController',
       '/file/' => array(
         '(query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorFileListController',
+        'view/(?P<id>[^/]+)/'.
+          '(?:(?P<engineKey>[^/]+)/)?'.
+          '(?:\$(?P<lines>\d+(?:-\d+)?))?'
+          => 'PhabricatorFileViewController',
+        'info/(?P<phid>[^/]+)/' => 'PhabricatorFileViewController',
         'upload/' => 'PhabricatorFileUploadController',
         'dropupload/' => 'PhabricatorFileDropUploadController',
         'compose/' => 'PhabricatorFileComposeController',
@@ -80,7 +86,6 @@ final class PhabricatorFilesApplication extends PhabricatorApplication {
         'delete/(?P<id>[1-9]\d*)/' => 'PhabricatorFileDeleteController',
         $this->getEditRoutePattern('edit/')
           => 'PhabricatorFileEditController',
-        'info/(?P<phid>[^/]+)/' => 'PhabricatorFileInfoController',
         'imageproxy/' => 'PhabricatorFileImageProxyController',
         'transforms/(?P<id>[1-9]\d*)/' =>
           'PhabricatorFileTransformListController',

src/applications/files/controller/PhabricatorFileViewController.php

@@ -1,6 +1,6 @@
 <?php
 
-final class PhabricatorFileInfoController extends PhabricatorFileController {
+final class PhabricatorFileViewController extends PhabricatorFileController {
 
   public function shouldAllowPublic() {
     return true;
@@ -404,26 +404,36 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
 
   private function newFileContent(PhabricatorFile $file) {
     $viewer = $this->getViewer();
+    $request = $this->getRequest();
 
     $ref = id(new PhabricatorDocumentRef())
       ->setFile($file);
 
     $engines = PhabricatorDocumentEngine::getEnginesForRef($viewer, $ref);
-    $engine = head($engines);
 
-    $content = $engine->newDocument($ref);
+    $engine_key = $request->getURIData('engineKey');
+    if (!isset($engines[$engine_key])) {
+      $engine_key = head_key($engines);
+    }
+    $engine = $engines[$engine_key];
 
-    $icon = $engine->newDocumentIcon($ref);
+    $lines = $request->getURILineRange('lines', 1000);
+    if ($lines) {
+      $engine->setHighlightedLines(range($lines[0], $lines[1]));
+    }
 
     $views = array();
-    foreach ($engines as $candidate_engine) {
+    foreach ($engines as $candidate_key => $candidate_engine) {
       $label = $candidate_engine->getViewAsLabel($ref);
       if ($label === null) {
         continue;
       }
 
+      $view_uri = '/file/view/'.$file->getID().'/'.$candidate_key.'/';
+
       $view_icon = $candidate_engine->getViewAsIconIcon($ref);
       $view_color = $candidate_engine->getViewAsIconColor($ref);
+      $loading = $candidate_engine->newLoadingContent($ref);
 
       $views[] = array(
         'viewKey' => $candidate_engine->getDocumentEngineKey(),
@@ -431,12 +441,26 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
         'color' => $view_color,
         'name' => $label,
         'engineURI' => $candidate_engine->getRenderURI($ref),
+        'viewURI' => $view_uri,
+        'loadingMarkup' => hsprintf('%s', $loading),
       );
     }
 
-    Javelin::initBehavior('document-engine');
-
     $viewport_id = celerity_generate_unique_node_id();
+    $control_id = celerity_generate_unique_node_id();
+    $icon = $engine->newDocumentIcon($ref);
+
+    if ($engine->shouldRenderAsync($ref)) {
+      $content = $engine->newLoadingContent($ref);
+      $config = array(
+        'renderControlID' => $control_id,
+      );
+    } else {
+      $content = $engine->newDocument($ref);
+      $config = array();
+    }
+
+    Javelin::initBehavior('document-engine', $config);
 
     $viewport = phutil_tag(
       'div',
@@ -457,6 +481,7 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
       ->setText(pht('View Options'))
       ->setIcon('fa-file-image-o')
       ->setColor(PHUIButtonView::GREY)
+      ->setID($control_id)
       ->setMetadata($meta)
       ->setDropdown(true)
       ->addSigil('document-engine-view-dropdown');

src/applications/files/document/PhabricatorDocumentEngine.php

@@ -4,6 +4,7 @@ abstract class PhabricatorDocumentEngine
   extends Phobject {
 
   private $viewer;
+  private $highlightedLines = array();
 
   final public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -14,10 +15,23 @@ abstract class PhabricatorDocumentEngine
     return $this->viewer;
   }
 
+  final public function setHighlightedLines(array $highlighted_lines) {
+    $this->highlightedLines = $highlighted_lines;
+    return $this;
+  }
+
+  final public function getHighlightedLines() {
+    return $this->highlightedLines;
+  }
+
   final public function canRenderDocument(PhabricatorDocumentRef $ref) {
     return $this->canRenderDocumentType($ref);
   }
 
+  public function shouldRenderAsync(PhabricatorDocumentRef $ref) {
+    return false;
+  }
+
   abstract protected function canRenderDocumentType(
     PhabricatorDocumentRef $ref);
 
@@ -49,6 +63,10 @@ abstract class PhabricatorDocumentEngine
     return 'fa-file-o';
   }
 
+  protected function getDocumentRenderingText(PhabricatorDocumentRef $ref) {
+    return pht('Loading...');
+  }
+
   final public function getDocumentEngineKey() {
     return $this->getPhobjectClassConstant('ENGINEKEY');
   }
@@ -177,4 +195,20 @@ abstract class PhabricatorDocumentEngine
       $message);
   }
 
+  final public function newLoadingContent(PhabricatorDocumentRef $ref) {
+    $spinner = id(new PHUIIconView())
+      ->setIcon('fa-gear')
+      ->addClass('ph-spin');
+
+    return phutil_tag(
+      'div',
+      array(
+        'class' => 'document-engine-loading',
+      ),
+      array(
+        $spinner,
+        $this->getDocumentRenderingText($ref),
+      ));
+  }
+
 }

src/applications/files/document/PhabricatorJupyterDocumentEngine.php

@@ -13,6 +13,14 @@ final class PhabricatorJupyterDocumentEngine
     return 'fa-sun-o';
   }
 
+  protected function getDocumentRenderingText(PhabricatorDocumentRef $ref) {
+    return pht('Rendering Jupyter Notebook...');
+  }
+
+  public function shouldRenderAsync(PhabricatorDocumentRef $ref) {
+    return true;
+  }
+
   protected function getContentScore(PhabricatorDocumentRef $ref) {
     $name = $ref->getName();
 

src/applications/files/document/PhabricatorTextDocumentEngine.php

@@ -11,8 +11,8 @@ abstract class PhabricatorTextDocumentEngine
     $lines = phutil_split_lines($content);
 
     $view = id(new PhabricatorSourceCodeView())
-      ->setLines($lines)
-      ->disableHighlightOnClick();
+      ->setHighlights($this->getHighlightedLines())
+      ->setLines($lines);
 
     $container = phutil_tag(
       'div',

src/view/layout/PhabricatorSourceCodeView.php

@@ -85,7 +85,11 @@ final class PhabricatorSourceCodeView extends AphrontView {
       }
 
       if ($this->canClickHighlight) {
-        $line_href = $base_uri.'$'.$line_number;
+        if ($base_uri) {
+          $line_href = $base_uri.'$'.$line_number;
+        } else {
+          $line_href = null;
+        }
 
         $tag_number = phutil_tag(
           'a',

webroot/rsrc/css/phui/phui-property-list-view.css

@@ -267,6 +267,23 @@ div.phui-property-list-stacked .phui-property-list-properties
   margin: 20px;
 }
 
+.document-engine-in-flight {
+  opacity: 0.25;
+}
+
+.document-engine-loading {
+  margin: 20px;
+  text-align: center;
+  color: {$lightgreytext};
+}
+
+.document-engine-loading .phui-icon-view {
+  display: block;
+  font-size: 48px;
+  color: {$lightgreyborder};
+  padding: 8px;
+}
+
 .jupyter-cell-raw {
   white-space: pre-wrap;
   background: {$lightgreybackground};

webroot/rsrc/js/application/files/behavior-document-engine.js

@@ -5,7 +5,9 @@
  *           javelin-stratcom
  */
 
-JX.behavior('document-engine', function() {
+JX.behavior('document-engine', function(config, statics) {
+
+
 
   function onmenu(e) {
     var node = e.getNode('document-engine-view-dropdown');
@@ -21,6 +23,7 @@ JX.behavior('document-engine', function() {
     var list = new JX.PHUIXActionListView();
 
     var view;
+    var engines = [];
     for (var ii = 0; ii < data.views.length; ii++) {
       var spec = data.views[ii];
 
@@ -38,31 +41,103 @@ JX.behavior('document-engine', function() {
         e.prevent();
         menu.close();
 
-        onview(data, spec);
+        onview(data, spec, false);
       }, spec));
 
       list.addItem(view);
+
+      engines.push({
+        spec: spec,
+        view: view
+      });
     }
 
     menu.setContent(list.getNode());
 
+    menu.listen('open', function() {
+      for (var ii = 0; ii < engines.length; ii++) {
+        var engine = engines[ii];
+
+        // Highlight the current rendering engine.
+        var is_selected = (engine.spec.viewKey == data.viewKey);
+        engine.view.setSelected(is_selected);
+      }
+    });
+
     data.menu = menu;
     menu.open();
   }
 
-  function onview(data, spec) {
-    var handler = JX.bind(null, onrender, data);
+  function onview(data, spec, immediate) {
+    data.sequence = (data.sequence || 0) + 1;
+    var handler = JX.bind(null, onrender, data, data.sequence);
+
+    data.viewKey = spec.viewKey;
+    JX.History.replace(spec.viewURI);
 
     new JX.Request(spec.engineURI, handler)
       .send();
+
+    if (data.loadingView) {
+      // If we're already showing "Loading...", immediately change it to
+      // show the new document type.
+      onloading(data, spec);
+    } else if (!immediate) {
+      // Otherwise, grey out the document and show "Loading..." after a
+      // short delay. This prevents the content from flickering when rendering
+      // is fast.
+      var viewport = JX.$(data.viewportID);
+      JX.DOM.alterClass(viewport, 'document-engine-in-flight', true);
+
+      var load = JX.bind(null, onloading, data, spec);
+      data.loadTimer = setTimeout(load, 333);
+    }
   }
 
-  function onrender(data, r) {
+  function onloading(data, spec) {
+    data.loadingView = true;
+
     var viewport = JX.$(data.viewportID);
+    JX.DOM.alterClass(viewport, 'document-engine-in-flight', false);
+    JX.DOM.setContent(viewport, JX.$H(spec.loadingMarkup));
+  }
+
+  function onrender(data, sequence, r) {
+    // If this isn't the most recent request we sent, throw it away. This can
+    // happen if the user makes multiple selections from the menu while we are
+    // still rendering the first view.
+    if (sequence != data.sequence) {
+      return;
+    }
+
+    if (data.loadTimer) {
+      clearTimeout(data.loadTimer);
+      data.loadTimer = null;
+    }
+
+    var viewport = JX.$(data.viewportID);
+
+    JX.DOM.alterClass(viewport, 'document-engine-in-flight', false);
+    data.loadingView = false;
 
     JX.DOM.setContent(viewport, JX.$H(r.markup));
   }
 
-  JX.Stratcom.listen('click', 'document-engine-view-dropdown', onmenu);
+  if (!statics.initialized) {
+    JX.Stratcom.listen('click', 'document-engine-view-dropdown', onmenu);
+    statics.initialized = true;
+  }
+
+  if (config.renderControlID) {
+    var control = JX.$(config.renderControlID);
+    var data = JX.Stratcom.getData(control);
+
+    for (var ii = 0; ii < data.views.length; ii++) {
+      if (data.views[ii].viewKey == data.viewKey) {
+        onview(data, data.views[ii], true);
+        break;
+      }
+    }
+  }
 
 });

webroot/rsrc/js/core/behavior-line-linker.js

@@ -145,6 +145,10 @@ JX.behavior('phabricator-line-linker', function() {
       var t = getRowNumber(target);
       var uri = JX.Stratcom.getData(root).uri;
 
+      if (!uri) {
+        uri = ('' + window.location).split('$')[0];
+      }
+
       origin = null;
       target = null;
       root = null;