render_tag -> tag: PropertyListView

Summary: Converts callsites in PropertyListView (addDetail() and setTextContent()).

Test Plan: Grepped for PhabricatorPropertyListView, addDetail() and setTextContent().

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

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

src/applications/base/controller/PhabricatorController.php

@@ -276,7 +276,8 @@ abstract class PhabricatorController extends AphrontController {
     foreach ($phids as $phid) {
       $items[] = $this->getHandle($phid)->renderLink();
     }
-    return implode($style_map[$style], $items);
+
+    return phutil_safe_html(implode($style_map[$style], $items));
   }
 
   protected function buildApplicationMenu() {

src/applications/config/controller/PhabricatorConfigAllController.php

@@ -68,13 +68,16 @@ final class PhabricatorConfigAllController
       $display_version = pht('Unknown');
     }
     $version_property_list = id(new PhabricatorPropertyListView());
-    $version_property_list->addProperty('Version',
+    $version_property_list->addProperty(
-      phutil_escape_html($display_version));
+      pht('Version'),
+      $display_version);
+
     $version_path = $phabricator_root.'/conf/local/VERSION';
     if (Filesystem::pathExists($version_path)) {
       $version_from_file = Filesystem::readFile($version_path);
-      $version_property_list->addProperty('Local Version',
+      $version_property_list->addProperty(
-        phutil_escape_html($version_from_file));
+        pht('Local Version'),
+        $version_from_file);
     }
 
     $nav = $this->buildSideNavView();

src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php

@@ -124,7 +124,7 @@ final class PhabricatorWorkerTaskDetailController
 
     $view->addProperty(
       pht('Task Class'),
-      phutil_escape_html($task->getTaskClass()));
+      $task->getTaskClass());
 
     if ($task->getLeaseExpires()) {
       if ($task->getLeaseExpires() > time()) {
@@ -133,7 +133,7 @@ final class PhabricatorWorkerTaskDetailController
         $lease_status = pht('Lease Expired');
       }
     } else {
-      $lease_status = '<em>'.pht('Not Leased').'</em>';
+      $lease_status = phutil_tag('em', array(), pht('Not Leased'));
     }
 
     $view->addProperty(
@@ -143,14 +143,14 @@ final class PhabricatorWorkerTaskDetailController
     $view->addProperty(
       pht('Lease Owner'),
       $task->getLeaseOwner()
-        ? phutil_escape_html($task->getLeaseOwner())
+        ? $task->getLeaseOwner()
-        : '<em>'.pht('None').'</em>');
+        : phutil_tag('em', array(), pht('None')));
 
     if ($task->getLeaseExpires() && $task->getLeaseOwner()) {
       $expires = ($task->getLeaseExpires() - time());
       $expires = phabricator_format_relative_time_detailed($expires);
     } else {
-      $expires = '<em>'.pht('None').'</em>';
+      $expires = phutil_tag('em', array(), pht('None'));
     }
 
     $view->addProperty(
@@ -159,12 +159,12 @@ final class PhabricatorWorkerTaskDetailController
 
     $view->addProperty(
       pht('Failure Count'),
-      phutil_escape_html($task->getFailureCount()));
+      $task->getFailureCount());
 
     if ($task->isArchived()) {
-      $duration = phutil_escape_html(number_format($task->getDuration()).' us');
+      $duration = number_format($task->getDuration()).' us';
     } else {
-      $duration = '<em>'.pht('Not Completed').'</em>';
+      $duration = phutil_tag('em', array(), pht('Not Completed'));
     }
 
     $view->addProperty(

src/applications/differential/field/specification/DifferentialApplyPatchFieldSpecification.php

@@ -13,7 +13,7 @@ final class DifferentialApplyPatchFieldSpecification
 
   public function renderValueForRevisionView() {
     $revision = $this->getRevision();
-    return '<tt>arc patch D'.$revision->getID().'</tt>';
+    return phutil_tag('tt', array(), 'arc patch D'.$revision->getID());
   }
 
 }

src/applications/differential/field/specification/DifferentialArcanistProjectFieldSpecification.php

@@ -27,7 +27,7 @@ final class DifferentialArcanistProjectFieldSpecification
     }
 
     $handle = $this->getHandle($arcanist_phid);
-    return phutil_escape_html($handle->getName());
+    return $handle->getName();
   }
 
   private function getArcanistProjectPHID() {

src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php

@@ -26,7 +26,7 @@ final class DifferentialBranchFieldSpecification
       return null;
     }
 
-    return phutil_escape_html($branch);
+    return $branch;
   }
 
   public function renderValueForMail($phase) {

src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php

@@ -26,7 +26,7 @@ final class DifferentialCommitsFieldSpecification
       $links[] = $this->getHandle($commit_phid)->renderLink();
     }
 
-    return implode('<br />', $links);
+    return array_interleave(phutil_tag('br'), $links);
   }
 
   private function getCommitPHIDs() {

src/applications/differential/field/specification/DifferentialDependenciesFieldSpecification.php

@@ -26,7 +26,7 @@ final class DifferentialDependenciesFieldSpecification
       $links[] = $this->getHandle($revision_phids)->renderLink();
     }
 
-    return implode('<br />', $links);
+    return array_interleave(phutil_tag('br'), $links);
   }
 
   private function getDependentRevisionPHIDs() {

src/applications/differential/field/specification/DifferentialDependsOnFieldSpecification.php

@@ -26,7 +26,7 @@ final class DifferentialDependsOnFieldSpecification
       $links[] = $this->getHandle($revision_phids)->renderLink();
     }
 
-    return implode('<br />', $links);
+    return phutil_safe_html(implode('<br />', $links));
   }
 
   private function getDependentRevisionPHIDs() {

src/applications/differential/field/specification/DifferentialExportPatchFieldSpecification.php

@@ -13,7 +13,10 @@ final class DifferentialExportPatchFieldSpecification
 
   public function renderValueForRevisionView() {
     $revision = $this->getRevision();
-    return '<tt>arc export --revision '.$revision->getID().'</tt>';
+    return phutil_tag(
+      'tt',
+      array(),
+      'arc export --revision '.$revision->getID());
   }
 
 }

src/applications/differential/field/specification/DifferentialFieldSpecification.php

@@ -274,7 +274,7 @@ abstract class DifferentialFieldSpecification {
    */
   public function renderUserList(array $user_phids) {
     if (!$user_phids) {
-      return '<em>None</em>';
+      return phutil_tag('em', array(), pht('None'));
     }
 
     $links = array();
@@ -283,7 +283,7 @@ abstract class DifferentialFieldSpecification {
       $links[] = $handle->renderLink();
     }
 
-    return implode(', ', $links);
+    return phutil_safe_html(implode(', ', $links));
   }
 
 

src/applications/differential/field/specification/DifferentialHostFieldSpecification.php

@@ -17,7 +17,7 @@ final class DifferentialHostFieldSpecification
     if (!$host) {
       return null;
     }
-    return phutil_escape_html($host);
+    return $host;
   }
 
 }

src/applications/differential/field/specification/DifferentialLinesFieldSpecification.php

@@ -13,7 +13,7 @@ final class DifferentialLinesFieldSpecification
 
   public function renderValueForRevisionView() {
     $diff = $this->getDiff();
-    return phutil_escape_html(number_format($diff->getLineCount()));
+    return number_format($diff->getLineCount());
   }
 
   public function shouldAppearOnRevisionList() {

src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php

@@ -29,7 +29,7 @@ final class DifferentialManiphestTasksFieldSpecification
       $links[] = $this->getHandle($task_phid)->renderLink();
     }
 
-    return implode('<br />', $links);
+    return phutil_safe_html(implode('<br />', $links));
   }
 
   private function getManiphestTaskPHIDs() {

src/applications/differential/field/specification/DifferentialPathFieldSpecification.php

@@ -19,7 +19,7 @@ final class DifferentialPathFieldSpecification
       return null;
     }
 
-    return phutil_escape_html($path);
+    return $path;
   }
 
 }

src/applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php

@@ -47,7 +47,7 @@ final class DifferentialRevertPlanFieldSpecification
     if (!$this->value) {
       return null;
     }
-    return phutil_escape_html($this->value);
+    return $this->value;
   }
 
   public function shouldAppearOnConduitView() {

src/applications/differential/view/DifferentialResultsTableView.php

@@ -102,13 +102,13 @@ final class DifferentialResultsTableView extends AphrontView {
 
     require_celerity_resource('differential-results-table-css');
 
-    return javelin_render_tag(
+    return javelin_tag(
       'table',
       array(
         'class' => 'differential-results-table',
         'sigil' => 'differential-results-table',
       ),
-      implode("\n", $rows));
+      $rows);
   }
 
 

src/applications/differential/view/DifferentialRevisionDetailView.php

@@ -66,10 +66,10 @@ final class DifferentialRevisionDetailView extends AphrontView {
       switch ($local_vcs) {
         case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-          $next_step = '<tt>arc land</tt>';
+          $next_step = phutil_tag('tt', array(), 'arc land');
           break;
         case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-          $next_step = '<tt>arc commit</tt>';
+          $next_step = phutil_tag('tt', array(), 'arc commit');
           break;
       }
     }

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -805,17 +805,17 @@ final class DiffusionBrowseFileController extends DiffusionController {
     $size = strlen($data);
     $properties->addTextContent(
       pht('This is a binary file. It is %2$s byte(s) in length.',
-          $size,
+        $size,
-          PhutilTranslator::getInstance()->formatNumber($size))
+        PhutilTranslator::getInstance()->formatNumber($size)));
-    );
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($this->getRequest()->getUser())
       ->addAction($this->createEditAction())
-      ->addAction(id(new PhabricatorActionView())
+      ->addAction(
-                    ->setName(pht('Download Binary File...'))
+        id(new PhabricatorActionView())
-                    ->setIcon('download')
+          ->setName(pht('Download Binary File...'))
-                    ->setHref($file_uri));
+          ->setIcon('download')
+          ->setHref($file_uri));
 
     return array($actions, $properties);
 

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -92,11 +92,13 @@ final class DiffusionCommitController extends DiffusionController {
       }
 
       $property_list->addTextContent(
-        '<div class="diffusion-commit-message phabricator-remarkup">'.
+        phutil_tag(
+          'div',
+          array(
+            'class' => 'diffusion-commit-message phabricator-remarkup',
+          ),
           phutil_safe_html(
-            $engine->markupText($commit_data->getCommitMessage())).
+            $engine->markupText($commit_data->getCommitMessage()))));
-        '</div>'
-      );
 
       $content[] = $top_anchor;
       $content[] = $headsup_view;
@@ -412,7 +414,7 @@ final class DiffusionCommitController extends DiffusionController {
     if ($data->getCommitDetail('authorPHID')) {
       $props['Author'] = $handles[$author_phid]->renderLink();
     } else {
-      $props['Author'] = phutil_escape_html($data->getAuthorName());
+      $props['Author'] = $data->getAuthorName();
     }
 
     $reviewer_phid = $data->getCommitDetail('reviewerPHID');
@@ -426,7 +428,7 @@ final class DiffusionCommitController extends DiffusionController {
       if ($data->getCommitDetail('committerPHID')) {
         $props['Committer'] = $handles[$committer_phid]->renderLink();
       } else {
-        $props['Committer'] = phutil_escape_html($committer);
+        $props['Committer'] = $committer;
       }
     }
 
@@ -467,7 +469,7 @@ final class DiffusionCommitController extends DiffusionController {
       foreach ($task_phids as $phid) {
         $task_list[] = $handles[$phid]->renderLink();
       }
-      $task_list = implode('<br />', $task_list);
+      $task_list = array_interleave(phutil_tag('br'), $task_list);
       $props['Tasks'] = $task_list;
     }
 
@@ -476,7 +478,7 @@ final class DiffusionCommitController extends DiffusionController {
       foreach ($proj_phids as $phid) {
         $proj_list[] = $handles[$phid]->renderLink();
       }
-      $proj_list = implode('<br />', $proj_list);
+      $proj_list = array_interleave(phutil_tag('br'), $proj_list);
       $props['Projects'] = $proj_list;
     }
 

src/applications/drydock/controller/DrydockLeaseViewController.php

@@ -109,19 +109,17 @@ final class DrydockLeaseViewController extends DrydockController {
 
     $view->addProperty(
       pht('Resource Type'),
-      phutil_escape_html($lease->getResourceType()));
+      $lease->getResourceType());
 
     $view->addProperty(
       pht('Resource'),
-      phutil_escape_html($lease->getResourceID()));
+      $lease->getResourceID());
 
     $attributes = $lease->getAttributes();
     if ($attributes) {
       $view->addSectionHeader(pht('Attributes'));
       foreach ($attributes as $key => $value) {
-        $view->addProperty(
+        $view->addProperty($key, $value);
-          phutil_escape_html($key),
-          phutil_escape_html($value));
       }
     }
 

src/applications/drydock/controller/DrydockResourceViewController.php

@@ -97,7 +97,6 @@ final class DrydockResourceViewController extends DrydockController {
 
     $status = $resource->getStatus();
     $status = DrydockResourceStatus::getNameForStatus($status);
-    $status = phutil_escape_html($status);
 
     $view->addProperty(
       pht('Status'),
@@ -105,15 +104,13 @@ final class DrydockResourceViewController extends DrydockController {
 
     $view->addProperty(
       pht('Resource Type'),
-      phutil_escape_html($resource->getType()));
+      $resource->getType());
 
     $attributes = $resource->getAttributes();
     if ($attributes) {
       $view->addSectionHeader(pht('Attributes'));
       foreach ($attributes as $key => $value) {
-        $view->addProperty(
+        $view->addProperty($key, $value);
-          phutil_escape_html($key),
-          phutil_escape_html($value));
       }
     }
 

src/applications/files/controller/PhabricatorFileInfoController.php

@@ -112,19 +112,19 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
 
     $view->addProperty(
       pht('Mime Type'),
-      phutil_escape_html($file->getMimeType()));
+      $file->getMimeType());
 
     $view->addProperty(
       pht('Engine'),
-      phutil_escape_html($file->getStorageEngine()));
+      $file->getStorageEngine());
 
     $view->addProperty(
       pht('Format'),
-      phutil_escape_html($file->getStorageFormat()));
+      $file->getStorageFormat());
 
     $view->addProperty(
       pht('Handle'),
-      phutil_escape_html($file->getStorageHandle()));
+      $file->getStorageHandle());
 
     $metadata = $file->getMetadata();
     if (!empty($metadata)) {
@@ -133,7 +133,7 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
       foreach ($metadata as $key => $value) {
         $view->addProperty(
           PhabricatorFile::getMetadataName($key),
-          phutil_escape_html($value));
+          $value);
       }
     }
 

src/applications/macro/controller/PhabricatorMacroViewController.php

@@ -147,9 +147,9 @@ final class PhabricatorMacroViewController
       foreach ($subscribers as $subscriber) {
         $sub_view[] = $this->getHandle($subscriber)->renderLink();
       }
-      $sub_view = implode(', ', $sub_view);
+      $sub_view = array_interleave(', ', $sub_view);
     } else {
-      $sub_view = '<em>'.pht('None').'</em>';
+      $sub_view = phutil_tag('em', array(), pht('None'));
     }
 
     $view->addProperty(

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -435,18 +435,17 @@ final class ManiphestTaskDetailController extends ManiphestController {
       pht('Assigned To'),
       $task->getOwnerPHID()
         ? $this->getHandle($task->getOwnerPHID())->renderLink()
-        : '<em>'.pht('None').'</em>');
+        : phutil_tag('em', array(), pht('None')));
 
     $view->addProperty(
       pht('Priority'),
-      phutil_escape_html(
+      ManiphestTaskPriority::getTaskPriorityName($task->getPriority()));
-        ManiphestTaskPriority::getTaskPriorityName($task->getPriority())));
 
     $view->addProperty(
       pht('Subscribers'),
       $task->getCCPHIDs()
         ? $this->renderHandlesForPHIDs($task->getCCPHIDs(), ',')
-        : '<em>'.pht('None').'</em>');
+        : phutil_tag('em', array(), pht('None')));
 
     $view->addProperty(
       pht('Author'),
@@ -469,7 +468,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
       pht('Projects'),
       $task->getProjectPHIDs()
         ? $this->renderHandlesForPHIDs($task->getProjectPHIDs(), ',')
-        : '<em>'.pht('None').'</em>');
+        : phutil_tag('em', array(), pht('None')));
 
     foreach ($aux_fields as $aux_field) {
       $aux_key = $aux_field->getAuxiliaryKey();

src/applications/phame/controller/blog/PhameBlogViewController.php

@@ -69,11 +69,11 @@ final class PhameBlogViewController extends PhameController {
 
     $properties->addProperty(
       pht('Skin'),
-      phutil_escape_html($blog->getSkin()));
+      $blog->getSkin());
 
     $properties->addProperty(
       pht('Domain'),
-      phutil_escape_html($blog->getDomain()));
+      $blog->getDomain());
 
     $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
       $user,
@@ -97,9 +97,12 @@ final class PhameBlogViewController extends PhameController {
       ->process();
 
     $properties->addTextContent(
-      '<div class="phabricator-remarkup">'.
+      phutil_tag(
-        $engine->getOutput($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION).
+        'div',
-      '</div>');
+        array(
+          'class' => 'phabricator-remarkup',
+        ),
+        $engine->getOutput($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION)));
 
     return $properties;
   }

src/applications/phame/controller/post/PhamePostViewController.php

@@ -184,9 +184,12 @@ final class PhamePostViewController extends PhameController {
       ->process();
 
     $properties->addTextContent(
-      '<div class="phabricator-remarkup">'.
+      phutil_tag(
-        $engine->getOutput($post, PhamePost::MARKUP_FIELD_BODY).
+         'div',
-      '</div>');
+        array(
+          'class' => 'phabricator-remarkup',
+        ),
+        $engine->getOutput($post, PhamePost::MARKUP_FIELD_BODY)));
 
     return $properties;
   }

src/applications/pholio/controller/PholioMockViewController.php

@@ -148,7 +148,7 @@ final class PholioMockViewController extends PholioController {
       }
       $sub_view = implode(', ', $sub_view);
     } else {
-      $sub_view = '<em>'.pht('None').'</em>';
+      $sub_view = phutil_tag('em', array(), pht('None'));
     }
 
     $properties->addProperty(

src/applications/ponder/controller/PonderQuestionViewController.php

@@ -117,15 +117,12 @@ final class PonderQuestionViewController extends PonderController {
       phabricator_datetime($question->getDateCreated(), $viewer));
 
     if ($subscribers) {
-      foreach ($subscribers as $key => $subscriber) {
+      $subscribers = $this->renderHandlesForPHIDs($subscribers);
-        $subscribers[$key] = $this->getHandle($subscriber)->renderLink();
-      }
-      $subscribers = implode(', ', $subscribers);
     }
 
     $view->addProperty(
       pht('Subscribers'),
-      nonempty($subscribers, '<em>'.pht('None').'</em>'));
+      nonempty($subscribers, phutil_tag('em', array(), pht('None'))));
 
     return $view;
   }

src/applications/uiexample/examples/PhabricatorPropertyListExample.php

@@ -49,9 +49,10 @@ final class PhabricatorPropertyListExample extends PhabricatorUIExample {
     $view->addSectionHeader('Haiku About Pasta');
 
     $view->addTextContent(
-      'this is a pasta<br />'.
+      phutil_safe_html(
-      'haiku. it is very bad.<br />'.
+        'this is a pasta<br />'.
-      'what did you expect?');
+        'haiku. it is very bad.<br />'.
+        'what did you expect?'));
 
     $edge_cases_header = id(new PhabricatorHeaderView())
       ->setHeader(pht('Edge Cases'));

src/view/layout/PhabricatorPropertyListView.php

@@ -93,30 +93,21 @@ final class PhabricatorPropertyListView extends AphrontView {
           'class' => 'phabricator-property-list-key',
         ),
         $key);
-      $items[] = phutil_render_tag(
+
+      $items[] = phutil_tag(
         'dd',
         array(
           'class' => 'phabricator-property-list-value',
         ),
-        $this->renderSingleView($value));
+        $this->renderHTMLView($value));
     }
 
-    $list = phutil_render_tag(
+    $list = phutil_tag(
       'dl',
       array(
         'class' => 'phabricator-property-list-properties',
       ),
-      $this->renderSingleView($items));
+      $this->renderHTMLView($items));
-
-    $content = $this->renderChildren();
-    if (strlen($content)) {
-      $content = phutil_render_tag(
-        'div',
-        array(
-          'class' => 'phabricator-property-list-content',
-        ),
-        $content);
-    }
 
     $shortcuts = null;
     if ($this->hasKeyboardShortcuts) {
@@ -124,9 +115,9 @@ final class PhabricatorPropertyListView extends AphrontView {
         id(new AphrontKeyboardShortcutsAvailableView())->render();
     }
 
-    return
+    return array(
-      $shortcuts.
+      $shortcuts,
-      phutil_render_tag(
+      phutil_tag(
         'div',
         array(
           'class' => 'phabricator-property-list-container',
@@ -137,7 +128,7 @@ final class PhabricatorPropertyListView extends AphrontView {
             'div',
             array('class' => 'phabriator-property-list-view-end'),
             ''),
-        ));
+        )));
   }
 
   private function renderSectionPart(array $part) {
@@ -150,7 +141,7 @@ final class PhabricatorPropertyListView extends AphrontView {
   }
 
   private function renderTextPart(array $part) {
-    return phutil_render_tag(
+    return phutil_tag(
       'div',
       array(
         'class' => 'phabricator-property-list-text-content',