Reduce Pholio brokenness for non-image files

Summary:
Ref T5359. When users upload non-image file types (PDFs, text files, whatever), Pholio currently chokes in a few places. Make most of these behaviors more reasonable:

  - Provide thumbs in the required sizes.
  - Predict the thumb size of these files correctly.
  - Disable inline comments.
  - Make "View Fullsize" and "Download" into buttons. These mostly-work. Download should probaly really download, but CSRF on forms is a bit of a pain right now.

Test Plan: See screenshots.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5359

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

resources/celerity/map.php

@@ -80,8 +80,8 @@ return array(
     'rsrc/css/application/people/people-profile.css' => 'ba7b2762',
     'rsrc/css/application/phame/phame.css' => '19ecc703',
     'rsrc/css/application/pholio/pholio-edit.css' => 'b9e59b6d',
-    'rsrc/css/application/pholio/pholio-inline-comments.css' => '609c3320',
-    'rsrc/css/application/pholio/pholio.css' => '72af321e',
+    'rsrc/css/application/pholio/pholio-inline-comments.css' => '3d5a5590',
+    'rsrc/css/application/pholio/pholio.css' => '344c1440',
     'rsrc/css/application/phortune/phortune-credit-card-form.css' => 'b25b4beb',
     'rsrc/css/application/phrequent/phrequent.css' => 'ffc185ad',
     'rsrc/css/application/phriction/phriction-document-css.css' => '7d7f0071',
@@ -261,13 +261,21 @@ return array(
     'rsrc/image/icon/fatcow/source/mobile.png' => 'f1321264',
     'rsrc/image/icon/fatcow/source/tablet.png' => '49396799',
     'rsrc/image/icon/fatcow/source/web.png' => '136ccb5d',
+    'rsrc/image/icon/fatcow/thumbnails/default.p100.png' => '7d490b01',
     'rsrc/image/icon/fatcow/thumbnails/default160x120.png' => 'f2e8a2eb',
+    'rsrc/image/icon/fatcow/thumbnails/default280x210.png' => '43e8926a',
     'rsrc/image/icon/fatcow/thumbnails/default60x45.png' => '0118abed',
+    'rsrc/image/icon/fatcow/thumbnails/image.p100.png' => 'da23cf97',
     'rsrc/image/icon/fatcow/thumbnails/image160x120.png' => '79bb556a',
+    'rsrc/image/icon/fatcow/thumbnails/image280x210.png' => '91ae054a',
     'rsrc/image/icon/fatcow/thumbnails/image60x45.png' => 'c5e1685e',
+    'rsrc/image/icon/fatcow/thumbnails/pdf.p100.png' => '87d5e065',
     'rsrc/image/icon/fatcow/thumbnails/pdf160x120.png' => 'ac9edbf5',
+    'rsrc/image/icon/fatcow/thumbnails/pdf280x210.png' => '1c585653',
     'rsrc/image/icon/fatcow/thumbnails/pdf60x45.png' => 'c0db4143',
+    'rsrc/image/icon/fatcow/thumbnails/zip.p100.png' => '6ea5aae4',
     'rsrc/image/icon/fatcow/thumbnails/zip160x120.png' => '75f9cd0f',
+    'rsrc/image/icon/fatcow/thumbnails/zip280x210.png' => 'dfda5b8e',
     'rsrc/image/icon/fatcow/thumbnails/zip60x45.png' => 'af11bf3e',
     'rsrc/image/icon/lightbox/close-2.png' => 'cc40e7c8',
     'rsrc/image/icon/lightbox/close-hover-2.png' => 'fb5d6d9e',
@@ -390,7 +398,7 @@ return array(
     'rsrc/js/application/passphrase/phame-credential-control.js' => '1e1c8a59',
     'rsrc/js/application/phame/phame-post-preview.js' => '61d927ec',
     'rsrc/js/application/pholio/behavior-pholio-mock-edit.js' => '1e1e8bb0',
-    'rsrc/js/application/pholio/behavior-pholio-mock-view.js' => '09c4fe2d',
+    'rsrc/js/application/pholio/behavior-pholio-mock-view.js' => 'd7f9b108',
     'rsrc/js/application/phortune/behavior-balanced-payment-form.js' => '3b3e1664',
     'rsrc/js/application/phortune/behavior-stripe-payment-form.js' => '1693a296',
     'rsrc/js/application/phortune/behavior-test-payment-form.js' => 'b3e5ee60',
@@ -614,7 +622,7 @@ return array(
     'javelin-behavior-phabricator-watch-anchor' => '06e05112',
     'javelin-behavior-phame-post-preview' => '61d927ec',
     'javelin-behavior-pholio-mock-edit' => '1e1e8bb0',
-    'javelin-behavior-pholio-mock-view' => '09c4fe2d',
+    'javelin-behavior-pholio-mock-view' => 'd7f9b108',
     'javelin-behavior-phui-object-box-tabs' => 'a3e2244e',
     'javelin-behavior-phui-timeline-dropdown-menu' => '4d94d9c3',
     'javelin-behavior-policy-control' => 'f3fef818',
@@ -740,9 +748,9 @@ return array(
     'phabricator-uiexample-reactor-sendproperties' => '551add57',
     'phabricator-zindex-css' => 'efb673ac',
     'phame-css' => '19ecc703',
-    'pholio-css' => '72af321e',
+    'pholio-css' => '344c1440',
     'pholio-edit-css' => 'b9e59b6d',
-    'pholio-inline-comments-css' => '609c3320',
+    'pholio-inline-comments-css' => '3d5a5590',
     'phortune-credit-card-form' => '2290aeef',
     'phortune-credit-card-form-css' => 'b25b4beb',
     'phrequent-css' => 'ffc185ad',
@@ -875,21 +883,6 @@ return array(
       7 => 'javelin-uri',
       8 => 'javelin-routable',
     ),
-    '09c4fe2d' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-util',
-      2 => 'javelin-stratcom',
-      3 => 'javelin-dom',
-      4 => 'javelin-vector',
-      5 => 'javelin-magical-init',
-      6 => 'javelin-request',
-      7 => 'javelin-history',
-      8 => 'javelin-workflow',
-      9 => 'javelin-mask',
-      10 => 'javelin-behavior-device',
-      11 => 'phabricator-keyboard-shortcut',
-    ),
     '0a3f3021' =>
     array(
       0 => 'javelin-behavior',
@@ -1270,6 +1263,11 @@ return array(
       2 => 'javelin-util',
       3 => 'phabricator-shaped-request',
     ),
+    '7319e029' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-dom',
+    ),
     '62e18640' =>
     array(
       0 => 'javelin-install',
@@ -1342,11 +1340,6 @@ return array(
       1 => 'javelin-stratcom',
       2 => 'javelin-dom',
     ),
-    '7319e029' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-dom',
-    ),
     '76f4ebed' =>
     array(
       0 => 'javelin-install',
@@ -1875,6 +1868,21 @@ return array(
       3 => 'javelin-dom',
       4 => 'phabricator-keyboard-shortcut',
     ),
+    'd7f9b108' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-util',
+      2 => 'javelin-stratcom',
+      3 => 'javelin-dom',
+      4 => 'javelin-vector',
+      5 => 'javelin-magical-init',
+      6 => 'javelin-request',
+      7 => 'javelin-history',
+      8 => 'javelin-workflow',
+      9 => 'javelin-mask',
+      10 => 'javelin-behavior-device',
+      11 => 'phabricator-keyboard-shortcut',
+    ),
     'd83a949c' =>
     array(
       0 => 'javelin-behavior',

src/applications/files/controller/PhabricatorFileTransformController.php

@@ -68,9 +68,6 @@ final class PhabricatorFileTransformController
       case 'preview-100':
         $xformed_file = $this->executePreviewTransform($file, 100);
         break;
-      case 'preview-140':
-        $xformed_file = $this->executePreviewTransform($file, 140);
-        break;
       case 'preview-220':
         $xformed_file = $this->executePreviewTransform($file, 220);
         break;
@@ -115,18 +112,24 @@ final class PhabricatorFileTransformController
     }
 
     switch ($this->transform) {
+      case 'thumb-280x210':
+        $suffix = '280x210';
+        break;
       case 'thumb-160x120':
         $suffix = '160x120';
         break;
       case 'thumb-60x45':
         $suffix = '60x45';
         break;
+      case 'preview-100':
+        $suffix = '.p100';
+        break;
       default:
         throw new Exception('Unsupported transformation type!');
     }
 
     $path = celerity_get_resource_uri(
-      "/rsrc/image/icon/fatcow/thumbnails/{$prefix}{$suffix}.png");
+      "rsrc/image/icon/fatcow/thumbnails/{$prefix}{$suffix}.png");
 
     return id(new AphrontRedirectResponse())
       ->setURI($path);

src/applications/files/storage/PhabricatorFile.php

@@ -544,12 +544,6 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return PhabricatorEnv::getCDNURI($path);
   }
 
-  public function getPreview140URI() {
-    $path = '/file/xform/preview-140/'.$this->getPHID().'/'
-      .$this->getSecretKey().'/';
-    return PhabricatorEnv::getCDNURI($path);
-  }
-
   public function getPreview220URI() {
     $path = '/file/xform/preview-220/'.$this->getPHID().'/'
       .$this->getSecretKey().'/';

src/applications/pholio/view/PholioMockImagesView.php

@@ -62,6 +62,11 @@ final class PholioMockImagesView extends AphrontView {
       $selected_id = head_key($ids);
     }
 
+    // TODO: We could maybe do a better job with tailoring this, which is the
+    // image shown on the review stage.
+    $nonimage_uri = celerity_get_resource_uri(
+      'rsrc/image/icon/fatcow/thumbnails/default.p100.png');
+
     foreach ($mock->getAllImages() as $image) {
       $file = $image->getFile();
       $metadata = $file->getMetadata();
@@ -72,13 +77,19 @@ final class PholioMockImagesView extends AphrontView {
       $images[] = array(
         'id' => $image->getID(),
         'fullURI' => $file->getBestURI(),
+        'stageURI' => ($file->isViewableImage()
+          ? $file->getBestURI()
+          : $nonimage_uri),
         'pageURI' => $this->getImagePageURI($image, $mock),
+        'downloadURI' => $file->getInfoURI(),
         'historyURI' => $history_uri,
         'width' => $x,
         'height' => $y,
         'title' => $image->getName(),
         'desc' => $image->getDescription(),
         'isObsolete' => (bool)$image->getIsObsolete(),
+        'isImage' => $file->isViewableImage(),
+        'isViewable' => $file->isViewableInBrowser(),
       );
     }
 
@@ -99,6 +110,8 @@ final class PholioMockImagesView extends AphrontView {
       'loggedIn' => $this->getUser()->isLoggedIn(),
       'logInLink' => (string) $login_uri,
       'navsequence' => $navsequence,
+      'fullIcon' => id(new PHUIIconView())->setIconFont('fa-arrows-alt'),
+      'downloadIcon' => id(new PHUIIconView())->setIconFont('fa-download'),
     );
     Javelin::initBehavior('pholio-mock-view', $config);
 

src/applications/pholio/view/PholioMockThumbGridView.php

@@ -101,9 +101,18 @@ final class PholioMockThumbGridView extends AphrontView {
   private function renderThumbnail(PholioImage $image) {
     $thumbfile = $image->getFile();
 
-    $dimensions = PhabricatorImageTransformer::getPreviewDimensions(
-      $thumbfile,
-      100);
+    if ($image->getFile()->isViewableImage()) {
+      $dimensions = PhabricatorImageTransformer::getPreviewDimensions(
+        $thumbfile,
+        100);
+    } else {
+      // If this is a PDF or a text file or something, we'll end up using a
+      // generic thumbnail which is always sized correctly.
+      $dimensions = array(
+        'sdx' => 100,
+        'sdy' => 100,
+      );
+    }
 
     $tag = phutil_tag(
       'img',

src/infrastructure/celerity/api.php

@@ -50,12 +50,9 @@ function celerity_generate_unique_node_id() {
  * @group celerity
  */
 function celerity_get_resource_uri($resource, $source = 'phabricator') {
+  $resource = ltrim($resource, '/');
+
   $map = CelerityResourceMap::getNamedInstance($source);
-
-  $uri = $map->getURIForName($resource);
-  if ($uri) {
-    return $uri;
-  }
-
-  return $resource;
+  $response = CelerityAPI::getStaticResourceResponse();
+  return $response->getURI($map, $resource);
 }

webroot/rsrc/css/application/pholio/pholio-inline-comments.css

@@ -11,6 +11,7 @@
   background: #fff;
   border-top: 1px solid {$thinblueborder};
   text-align: left;
+  overflow: hidden;
 }
 
 .pholio-mock-inline-comments a {

webroot/rsrc/css/application/pholio/pholio.css

@@ -167,3 +167,36 @@
   text-decoration: none;
   background: {$indigo};
 }
+
+.pholio-image-button {
+  float: right;
+  margin-left: 2px;
+}
+
+.pholio-image-button-link {
+  width: 56px;
+  height: 56px;
+  overflow: hidden;
+  display: block;
+  position: relative;
+  background: {$lightgreybackground};
+  text-align: center;
+  line-height: 56px;
+  font-size: 24px;
+}
+
+a.pholio-image-button-link:hover {
+  background: {$darkgreybackground};
+}
+
+span.pholio-image-button-link .phui-icon-view {
+  color: {$darkgreybackground};
+}
+
+a.pholio-image-button-link .phui-icon-view {
+  color: {$lightgreytext};
+}
+
+.device-desktop a.pholio-image-button-link:hover .phui-icon-view {
+  color: {$sky};
+}

webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js

@@ -206,7 +206,7 @@ JX.behavior('pholio-mock-view', function(config) {
 
     var img = JX.$N('img', {className: 'pholio-mock-image'});
     img.onload = JX.bind(img, onload_image, active_image.id);
-    img.src = active_image.fullURI;
+    img.src = active_image.stageURI;
 
     var thumbs = JX.DOM.scry(
       JX.$('pholio-mock-thumb-grid'),
@@ -260,6 +260,12 @@ JX.behavior('pholio-mock-view', function(config) {
 
     e.kill();
 
+    if (!active_image.isImage) {
+      // If this is a PDF or something like that, we eat the event but we
+      // don't let users add inlines to the thumbnail.
+      return;
+    }
+
     is_dragging = true;
     drag_begin = get_image_xy(JX.$V(e));
     drag_end = drag_begin;
@@ -583,6 +589,39 @@ JX.behavior('pholio-mock-view', function(config) {
   function render_image_info(image) {
     var info = [];
 
+    var buttons = [];
+
+    buttons.push(
+      JX.$N(
+        'div',
+        {
+          className: 'pholio-image-button'
+        },
+        JX.$N(
+          image.isViewable ? 'a' : 'span',
+          {
+            href: image.fullURI,
+            target: '_blank',
+            className: 'pholio-image-button-link'
+          },
+          JX.$H(config.fullIcon))));
+
+    // TODO: This should be a form which performs the download; for now, it
+    // just takes the user to the info page.
+    buttons.push(
+      JX.$N(
+        'div',
+        {
+          className: 'pholio-image-button'
+        },
+        JX.$N(
+          'a',
+          {
+            href: image.downloadURI,
+            className: 'pholio-image-button-link'
+          },
+          JX.$H(config.downloadIcon))));
+
     var title = JX.$N(
       'div',
       {className: 'pholio-image-title'},
@@ -604,18 +643,12 @@ JX.behavior('pholio-mock-view', function(config) {
       info.push(embed);
     }
 
-    var full_link = JX.$N(
-      'a',
-      {href: image.fullURI, target: '_blank'},
-      'View Full Image');
-    info.push(full_link);
-
     for (var ii = 0; ii < info.length; ii++) {
       info[ii] = JX.$N('div', {className: 'pholio-image-info-item'}, info[ii]);
     }
     info = JX.$N('div', {className: 'pholio-image-info'}, info);
 
-    return info;
+    return [buttons, info];
   }
 
   function render_reticle(classes) {
@@ -654,7 +687,7 @@ JX.behavior('pholio-mock-view', function(config) {
     var image = JX.$N('img');
     image.onload = lightbox_loaded;
     setTimeout(function() {
-      image.src = active_image.fullURI;
+      image.src = active_image.stageURI;
     }, 1000);
     JX.DOM.setContent(lightbox, image);
     JX.DOM.alterClass(lightbox, 'pholio-device-lightbox-loading', true);
@@ -694,7 +727,7 @@ JX.behavior('pholio-mock-view', function(config) {
 
   var preload = [];
   for (var ii = 0; ii < config.images.length; ii++) {
-    preload.push(config.images[ii].fullURI);
+    preload.push(config.images[ii].stageURI);
   }
 
   function preload_next() {