Provide default image alt text in more contexts and support custom alt text

Summary:
Ref T13629.

  - Allow files to have custom alt text.
  - If a file doesn't have alt text, try to generate a plausible default alt text with the information we have.

Test Plan:
  - Viewed image files in DocumentEngine diffs, files, `{Fxxx}` embeds, and lightboxes.
  - Saw default alt text in all cases, or custom alt text if provided.
  - Set, modified, and removed file alt text. Viewed timeline and feed.
  - Pulled alt text with "conduit.search".

Maniphest Tasks: T13629

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

resources/celerity/map.php

@@ -10,7 +10,7 @@ return array(
     'conpherence.pkg.css' => '0e3cf785',
     'conpherence.pkg.js' => '020aebcf',
     'core.pkg.css' => '0ae696de',
-    'core.pkg.js' => '079198f6',
+    'core.pkg.js' => 'ab3502fe',
     'dark-console.pkg.js' => '187792c2',
     'differential.pkg.css' => '5c459f92',
     'differential.pkg.js' => '5080baf4',
@@ -489,7 +489,7 @@ return array(
     'rsrc/js/core/behavior-hovercard.js' => '183738e6',
     'rsrc/js/core/behavior-keyboard-pager.js' => '1325b731',
     'rsrc/js/core/behavior-keyboard-shortcuts.js' => '42c44e8b',
-    'rsrc/js/core/behavior-lightbox-attachments.js' => 'c7e748bf',
+    'rsrc/js/core/behavior-lightbox-attachments.js' => '14c7ab36',
     'rsrc/js/core/behavior-line-linker.js' => '0d915ff5',
     'rsrc/js/core/behavior-linked-container.js' => '74446546',
     'rsrc/js/core/behavior-more.js' => '506aa3f4',
@@ -642,7 +642,7 @@ return array(
     'javelin-behavior-history-install' => '6a1583a8',
     'javelin-behavior-icon-composer' => '38a6cedb',
     'javelin-behavior-launch-icon-composer' => 'a17b84f1',
-    'javelin-behavior-lightbox-attachments' => 'c7e748bf',
+    'javelin-behavior-lightbox-attachments' => '14c7ab36',
     'javelin-behavior-line-chart' => 'ad258e28',
     'javelin-behavior-linked-container' => '74446546',
     'javelin-behavior-maniphest-batch-selector' => '139ef688',
@@ -1039,6 +1039,15 @@ return array(
       'javelin-stratcom',
       'javelin-util',
     ),
+    '14c7ab36' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-mask',
+      'javelin-util',
+      'phuix-icon-view',
+      'phabricator-busy',
+    ),
     '183738e6' => array(
       'javelin-behavior',
       'javelin-behavior-device',
@@ -2091,15 +2100,6 @@ return array(
       'javelin-workflow',
       'javelin-json',
     ),
-    'c7e748bf' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-mask',
-      'javelin-util',
-      'phuix-icon-view',
-      'phabricator-busy',
-    ),
     'cc2c5de5' => array(
       'javelin-install',
       'phuix-button-view',

src/__phutil_library_map__.php

@@ -3428,6 +3428,7 @@ phutil_register_library_map(array(
     'PhabricatorFerretSearchEngineExtension' => 'applications/search/engineextension/PhabricatorFerretSearchEngineExtension.php',
     'PhabricatorFile' => 'applications/files/storage/PhabricatorFile.php',
     'PhabricatorFileAES256StorageFormat' => 'applications/files/format/PhabricatorFileAES256StorageFormat.php',
+    'PhabricatorFileAltTextTransaction' => 'applications/files/xaction/PhabricatorFileAltTextTransaction.php',
     'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php',
     'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php',
     'PhabricatorFileChunkIterator' => 'applications/files/engine/PhabricatorFileChunkIterator.php',
@@ -9956,6 +9957,7 @@ phutil_register_library_map(array(
       'PhabricatorNgramsInterface',
     ),
     'PhabricatorFileAES256StorageFormat' => 'PhabricatorFileStorageFormat',
+    'PhabricatorFileAltTextTransaction' => 'PhabricatorFileTransactionType',
     'PhabricatorFileBundleLoader' => 'Phobject',
     'PhabricatorFileChunk' => array(
       'PhabricatorFileDAO',

src/applications/files/controller/PhabricatorFileViewController.php

@@ -310,6 +310,15 @@ final class PhabricatorFileViewController extends PhabricatorFileController {
       pht('Storage Handle'),
       $file->getStorageHandle());
 
+    $custom_alt = $file->getCustomAltText();
+    if (strlen($custom_alt)) {
+      $finfo->addProperty(pht('Custom Alt Text'), $custom_alt);
+    }
+
+    $default_alt = $file->getDefaultAltText();
+    if (strlen($default_alt)) {
+      $finfo->addProperty(pht('Default Alt Text'), $default_alt);
+    }
 
     $phids = $file->getObjectPHIDs();
     if ($phids) {

src/applications/files/document/PhabricatorImageDocumentEngine.php

@@ -86,6 +86,7 @@ final class PhabricatorImageDocumentEngine
       phutil_tag(
         'img',
         array(
+          'alt' => $file->getAltText(),
           'src' => $file->getBestURI(),
         )));
 
@@ -129,6 +130,7 @@ final class PhabricatorImageDocumentEngine
     $image = phutil_tag(
       'img',
       array(
+        'alt' => $file->getAltText(),
         'src' => $source_uri,
       ));
 

src/applications/files/editor/PhabricatorFileEditEngine.php

@@ -75,6 +75,14 @@ final class PhabricatorFileEditEngine
         ->setConduitDescription(pht('Rename the file.'))
         ->setConduitTypeDescription(pht('New file name.'))
         ->setValue($object->getName()),
+      id(new PhabricatorTextEditField())
+        ->setKey('alt')
+        ->setLabel(pht('Alt Text'))
+        ->setTransactionType(PhabricatorFileAltTextTransaction::TRANSACTIONTYPE)
+        ->setDescription(pht('Human-readable file description.'))
+        ->setConduitDescription(pht('Set the file alt text.'))
+        ->setConduitTypeDescription(pht('New alt text.'))
+        ->setValue($object->getCustomAltText()),
     );
   }
 

src/applications/files/markup/PhabricatorEmbedFileRemarkupRule.php

@@ -161,10 +161,17 @@ final class PhabricatorEmbedFileRemarkupRule
       }
     }
 
+    $alt = null;
     if (isset($options['alt'])) {
-      $attrs['alt'] = $options['alt'];
+      $alt = $options['alt'];
     }
 
+    if (!strlen($alt)) {
+      $alt = $file->getAltText();
+    }
+
+    $attrs['alt'] = $alt;
+
     $img = phutil_tag('img', $attrs);
 
     $embed = javelin_tag(
@@ -174,9 +181,10 @@ final class PhabricatorEmbedFileRemarkupRule
         'class'       => $image_class,
         'sigil'       => 'lightboxable',
         'meta'        => array(
-          'phid'     => $file->getPHID(),
-          'uri'      => $file->getBestURI(),
-          'dUri'     => $file->getDownloadURI(),
+          'phid' => $file->getPHID(),
+          'uri' => $file->getBestURI(),
+          'dUri' => $file->getDownloadURI(),
+          'alt' => $alt,
           'viewable' => true,
           'monogram' => $file->getMonogram(),
         ),

src/applications/files/storage/PhabricatorFile.php

@@ -41,6 +41,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
   const METADATA_STORAGE = 'storage';
   const METADATA_INTEGRITY = 'integrity';
   const METADATA_CHUNK = 'chunk';
+  const METADATA_ALT_TEXT = 'alt';
 
   const STATUS_ACTIVE = 'active';
   const STATUS_DELETED = 'deleted';
@@ -1274,6 +1275,72 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return idx($this->metadata, self::METADATA_IMAGE_WIDTH);
   }
 
+  public function getAltText() {
+    $alt = $this->getCustomAltText();
+
+    if (strlen($alt)) {
+      return $alt;
+    }
+
+    return $this->getDefaultAltText();
+  }
+
+  public function getCustomAltText() {
+    return idx($this->metadata, self::METADATA_ALT_TEXT);
+  }
+
+  public function setCustomAltText($value) {
+    $value = phutil_string_cast($value);
+
+    if (!strlen($value)) {
+      $value = null;
+    }
+
+    if ($value === null) {
+      unset($this->metadata[self::METADATA_ALT_TEXT]);
+    } else {
+      $this->metadata[self::METADATA_ALT_TEXT] = $value;
+    }
+
+    return $this;
+  }
+
+  public function getDefaultAltText() {
+    $parts = array();
+
+    $name = $this->getName();
+    if (strlen($name)) {
+      $parts[] = $name;
+    }
+
+    $stats = array();
+
+    $image_x = $this->getImageHeight();
+    $image_y = $this->getImageWidth();
+
+    if ($image_x && $image_y) {
+      $stats[] = pht(
+        "%d\xC3\x97%d px",
+        new PhutilNumber($image_x),
+        new PhutilNumber($image_y));
+    }
+
+    $bytes = $this->getByteSize();
+    if ($bytes) {
+      $stats[] = phutil_format_bytes($bytes);
+    }
+
+    if ($stats) {
+      $parts[] = pht('(%s)', implode(', ', $stats));
+    }
+
+    if (!$parts) {
+      return null;
+    }
+
+    return implode(' ', $parts);
+  }
+
   public function getCanCDN() {
     if (!$this->isViewableImage()) {
       return false;
@@ -1676,6 +1743,10 @@ final class PhabricatorFile extends PhabricatorFileDAO
       'uri' => PhabricatorEnv::getURI($this->getURI()),
       'dataURI' => $this->getCDNURI('data'),
       'size' => (int)$this->getByteSize(),
+      'alt' => array(
+        'custom' => $this->getCustomAltText(),
+        'default' => $this->getDefaultAltText(),
+      ),
     );
   }
 

src/applications/files/xaction/PhabricatorFileAltTextTransaction.php

@@ -0,0 +1,94 @@
+<?php
+
+final class PhabricatorFileAltTextTransaction
+  extends PhabricatorFileTransactionType {
+
+  const TRANSACTIONTYPE = 'file:alt';
+
+  public function generateOldValue($object) {
+    return $object->getCustomAltText();
+  }
+
+  public function generateNewValue($object, $value) {
+    $value = phutil_string_cast($value);
+
+    if (!strlen($value)) {
+      $value = null;
+    }
+
+    return $value;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setCustomAltText($value);
+  }
+
+  public function getTitle() {
+    $old_value = $this->getOldValue();
+    $new_value = $this->getNewValue();
+
+    if (!strlen($old_value)) {
+      return pht(
+        '%s set the alternate text for this file to %s.',
+        $this->renderAuthor(),
+        $this->renderNewValue());
+    } else if (!strlen($new_value)) {
+      return pht(
+        '%s removed the alternate text for this file (was %s).',
+        $this->renderAuthor(),
+        $this->renderOldValue());
+    } else {
+      return pht(
+        '%s changed the alternate text for this file from %s to %s.',
+        $this->renderAuthor(),
+        $this->renderOldValue(),
+        $this->renderNewValue());
+    }
+  }
+
+  public function getTitleForFeed() {
+    $old_value = $this->getOldValue();
+    $new_value = $this->getNewValue();
+
+    if (!strlen($old_value)) {
+      return pht(
+        '%s set the alternate text for %s to %s.',
+        $this->renderAuthor(),
+        $this->renderObject(),
+        $this->renderNewValue());
+    } else if (!strlen($new_value)) {
+      return pht(
+        '%s removed the alternate text for %s (was %s).',
+        $this->renderAuthor(),
+        $this->renderObject(),
+        $this->renderOldValue());
+    } else {
+      return pht(
+        '%s changed the alternate text for %s from %s to %s.',
+        $this->renderAuthor(),
+        $this->renderObject(),
+        $this->renderOldValue(),
+        $this->renderNewValue());
+    }
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $max_length = 1024;
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'File alternate text must not be longer than %s character(s).',
+            new PhutilNumber($max_length)));
+      }
+    }
+
+    return $errors;
+  }
+
+}

webroot/rsrc/js/core/behavior-lightbox-attachments.js

@@ -94,16 +94,12 @@ JX.behavior('lightbox-attachments', function() {
 
     if (target_data.viewable) {
       img_uri = target_data.uri;
-      var alt_name = '';
-      if (typeof target_data.name != 'undefined') {
-        alt_name = target_data.name;
-      }
 
       img =
         JX.$N('img',
           {
             className : 'loading',
-            alt : alt_name
+            alt : target_data.alt || null
           }
         );
     } else {