Fix some HTML problems

Summary: I'm too lazy to attaching them for diffs where they were introduced.

Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

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

src/applications/auth/controller/PhabricatorLDAPLoginController.php

@@ -134,6 +134,7 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
     $panel->appendChild('<h1>'.pht('LDAP login').'</h1>');
     $panel->appendChild($ldap_form);
 
+    $error_view = null;
     if (isset($errors) && count($errors) > 0) {
       $error_view = new AphrontErrorView();
       $error_view->setTitle(pht('Login Failed'));
@@ -142,7 +143,7 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
 
     return $this->buildStandardPageResponse(
       array(
-        isset($error_view) ? $error_view : null,
+        $error_view,
         $panel,
       ),
       array(

src/applications/auth/controller/PhabricatorLoginController.php

@@ -299,7 +299,7 @@ final class PhabricatorLoginController
     return $this->buildApplicationPage(
       array(
         $error_view,
-        $login_message,
+        phutil_safe_html($login_message),
         $panel,
       ),
       array(

src/applications/calendar/controller/PhabricatorCalendarBrowseController.php

@@ -55,9 +55,7 @@ final class PhabricatorCalendarBrowseController
     $nav->appendChild(
       array(
         $this->getNoticeView(),
-        '<div style="padding: 20px;">',
-          $month_view,
-        '</div>',
+        hsprintf('<div style="padding: 20px;">%s</div>', $month_view->render()),
       ));
 
     return $this->buildApplicationPage(

src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php

@@ -94,7 +94,6 @@ final class PhabricatorChatLogChannelLogController
     require_celerity_resource('phabricator-chatlog-css');
 
     $out = array();
-    $out[] = '<table class="phabricator-chat-log">';
     foreach ($blocks as $block) {
       $author = $block['author'];
       $author = phutil_utf8_shorten($author, 18);
@@ -122,7 +121,6 @@ final class PhabricatorChatLogChannelLogController
         ),
         array($author, $message, $timestamp));
     }
-    $out[] = '</table>';
 
     $form = id(new AphrontFormView())
       ->setUser($user)
@@ -140,12 +138,11 @@ final class PhabricatorChatLogChannelLogController
 
     return $this->buildStandardPageResponse(
       array(
-        '<div class="phabricator-chat-log-panel">',
-        $form,
-        '<br />',
-        implode("\n", $out),
-        $pager,
-        '</div>',
+        hsprintf(
+          '<div class="phabricator-chat-log-panel">%s<br />%s%s</div>',
+          $form,
+          phutil_tag('table', array('class' => 'phabricator-chat-log'), $out),
+          $pager),
       ),
       array(
         'title' => 'Channel Log',

src/applications/differential/view/DifferentialAddCommentView.php

@@ -155,35 +155,35 @@ final class DifferentialAddCommentView extends AphrontView {
         'inline'    => 'inline-comment-preview',
       ));
 
-    $warning_container = '<div id="warnings">';
+    $warning_container = array();
     foreach ($warnings as $warning) {
       if ($warning) {
-        $warning_container .= $warning->render();
+        $warning_container[] = $warning->render();
       }
     }
-    $warning_container .= '</div>';
 
     $header = id(new PhabricatorHeaderView())
       ->setHeader($is_serious ? pht('Add Comment') : pht('Leap Into Action'));
 
-    return
-      id(new PhabricatorAnchorView())
-        ->setAnchorName('comment')
-        ->setNavigationMarker(true)
-        ->render().
+    return hsprintf(
+      '%s'.
       '<div class="differential-add-comment-panel">'.
-        $header->render().
-        $form->render().
-        $warning_container.
+        '%s%s%s'.
         '<div class="aphront-panel-preview aphront-panel-flush">'.
           '<div id="comment-preview">'.
-            '<span class="aphront-panel-preview-loading-text">'.
-              pht('Loading comment preview...').
-            '</span>'.
+            '<span class="aphront-panel-preview-loading-text">%s</span>'.
           '</div>'.
           '<div id="inline-comment-preview">'.
           '</div>'.
         '</div>'.
-      '</div>';
+      '</div>',
+      id(new PhabricatorAnchorView())
+        ->setAnchorName('comment')
+        ->setNavigationMarker(true)
+        ->render(),
+      $header->render(),
+      $form->render(),
+      phutil_tag('div', array('id' => 'warnings'), $warning_container),
+      pht('Loading comment preview...'));
   }
 }

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -687,7 +687,7 @@ final class DiffusionCommitController extends DiffusionController {
       'inlineuri'  => '/diffusion/inline/preview/'.$commit->getPHID().'/',
     ));
 
-    $preview_panel =
+    $preview_panel = hsprintf(
       '<div class="aphront-panel-preview aphront-panel-flush">
         <div id="audit-preview">
           <div class="aphront-panel-preview-loading-text">
@@ -696,27 +696,24 @@ final class DiffusionCommitController extends DiffusionController {
         </div>
         <div id="inline-comment-preview">
         </div>
-      </div>';
+      </div>');
 
     // TODO: This is pretty awkward, unify the CSS between Diffusion and
     // Differential better.
     require_celerity_resource('differential-core-view-css');
 
-    return phutil_render_tag(
+    return phutil_tag(
       'div',
       array(
         'id' => $pane_id,
       ),
-      phutil_render_tag(
-        'div',
-        array(
-          'class' => 'differential-add-comment-panel',
-        ),
+      hsprintf(
+        '<div class="differential-add-comment-panel">%s%s%s</div>',
         id(new PhabricatorAnchorView())
           ->setAnchorName('comment')
           ->setNavigationMarker(true)
-          ->render().
-        $panel->render().
+          ->render(),
+        $panel->render(),
         $preview_panel));
   }
 

src/applications/diviner/controller/DivinerListController.php

@@ -50,10 +50,7 @@ final class DivinerListController extends PhabricatorController {
         $flavor);
     }
 
-    $out =
-      '<div class="aphront-directory-list">'.
-        implode("\n", $out).
-      '</div>';
+    $out = phutil_tag('div', array('class' => 'aphront-directory-list'), $out);
 
     return $this->buildApplicationPage(
       $out,

src/applications/herald/view/HeraldRuleEditHistoryView.php

@@ -32,14 +32,14 @@ final class HeraldRuleEditHistoryView extends AphrontView {
 
       switch ($edit->getAction()) {
         case 'create':
-          $details = "Created rule '{$rule_name}'.";
+          $details = pht("Created rule '%s'.", $rule_name);
           break;
         case 'delete':
-          $details = "Deleted rule '{$rule_name}'.";
+          $details = pht("Deleted rule '%s'.", $rule_name);
           break;
         case 'edit':
         default:
-          $details = "Edited rule '{$rule_name}'.";
+          $details = pht("Edited rule '%s'.", $rule_name);
           break;
       }
 

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -301,14 +301,13 @@ final class ManiphestTaskDetailController extends ManiphestController {
     $comment_header = id(new PhabricatorHeaderView())
       ->setHeader($is_serious ? pht('Add Comment') : pht('Weigh In'));
 
-    $preview_panel =
+    $preview_panel = hsprintf(
       '<div class="aphront-panel-preview">
         <div id="transaction-preview">
-          <div class="aphront-panel-preview-loading-text">
-            '.pht('Loading preview...').'
-          </div>
+          <div class="aphront-panel-preview-loading-text">%s</div>
         </div>
-      </div>';
+      </div>',
+      pht('Loading preview...'));
 
     $transaction_view = new ManiphestTransactionListView();
     $transaction_view->setTransactions($transactions);

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -512,7 +512,7 @@ final class ManiphestTaskEditController extends ManiphestController {
     $panel->appendChild($form);
     $panel->setNoBackground();
 
-    $description_preview_panel =
+    $description_preview_panel = hsprintf(
       '<div class="aphront-panel-preview aphront-panel-preview-full">
         <div class="maniphest-description-preview-header">
           Description Preview
@@ -522,7 +522,7 @@ final class ManiphestTaskEditController extends ManiphestController {
             Loading preview...
           </div>
         </div>
-      </div>';
+      </div>');
 
     Javelin::initBehavior(
       'maniphest-description-preview',
@@ -542,7 +542,7 @@ final class ManiphestTaskEditController extends ManiphestController {
       array(
         $error_view,
         $panel,
-        $description_preview_panel
+        $description_preview_panel,
       ),
       array(
         'title' => $header_name,

src/applications/notification/controller/PhabricatorNotificationListController.php

@@ -43,19 +43,16 @@ final class PhabricatorNotificationListController
 
     if ($notifications) {
       $builder = new PhabricatorNotificationBuilder($notifications);
-      $view = $builder->buildView();
+      $view = $builder->buildView()->render();
     } else {
-      $view =
-        '<div class="phabricator-notification no-notifications">'.
-          $no_data.
-        '</div>';
+      $view = hsprintf(
+        '<div class="phabricator-notification no-notifications">%s</div>',
+        $no_data);
     }
 
-    $view = array(
-      '<div class="phabricator-notification-list">',
-      $view,
-      '</div>',
-    );
+    $view = hsprintf(
+      '<div class="phabricator-notification-list">%s</div>',
+      $view);
 
     $panel = new AphrontPanelView();
     $panel->setHeader($header);

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

@@ -148,7 +148,7 @@ final class PhamePostEditController
         ->setValue($submit_button)
       );
 
-    $preview_panel =
+    $preview_panel = hsprintf(
       '<div class="aphront-panel-preview">
          <div class="phame-post-preview-header">
            Post Preview
@@ -158,7 +158,7 @@ final class PhamePostEditController
              Loading preview...
            </div>
          </div>
-       </div>';
+       </div>');
 
     require_celerity_resource('phame-css');
     Javelin::initBehavior(

src/applications/phriction/controller/PhrictionDiffController.php

@@ -155,28 +155,32 @@ final class PhrictionDiffController
         $link_r = pht('Most Recent Change');
       }
 
-      $navigation_table =
+      $navigation_table = hsprintf(
         '<table class="phriction-history-nav-table">
           <tr>
-            <td class="nav-prev">'.$link_l.'</td>
-            <td class="nav-next">'.$link_r.'</td>
+            <td class="nav-prev">%s</td>
+            <td class="nav-next">%s</td>
           </tr>
-        </table>';
+        </table>',
+        $link_l,
+        $link_r);
     }
 
 
 
-    $output =
+    $output = hsprintf(
       '<div class="phriction-document-history-diff">'.
-        $comparison_table->render().
-        '<br />'.
-        '<br />'.
-        $navigation_table.
+        '%s<br /><br />%s'.
         '<table class="phriction-revert-table">'.
-          '<tr><td>'.$revert_l.'</td><td>'.$revert_r.'</td>'.
+          '<tr><td>%s</td><td>%s</td>'.
         '</table>'.
-        $output.
-      '</div>';
+        '%s'.
+      '</div>',
+      $comparison_table->render(),
+      $navigation_table,
+      $revert_l,
+      $revert_r,
+      $output);
 
     return $this->buildStandardPageResponse(
       array(

src/applications/phriction/controller/PhrictionDocumentController.php

@@ -110,9 +110,9 @@ final class PhrictionDocumentController
 
       $project_info = null;
       if ($project_phid) {
-        $project_info =
-          '<br />This document is about the project '.
-          $handles[$project_phid]->renderLink().'.';
+        $project_info = hsprintf(
+          '<br />This document is about the project %s.',
+          $handles[$project_phid]->renderLink());
       }
 
       $index_link = phutil_tag(
@@ -122,12 +122,11 @@ final class PhrictionDocumentController
         ),
         pht('Document Index'));
 
-      $byline =
-        '<div class="phriction-byline">'.
-          "Last updated {$when} by ".
-          $handles[$content->getAuthorPHID()]->renderLink().'.'.
-          $project_info.
-        '</div>';
+      $byline = hsprintf(
+        '<div class="phriction-byline">Last updated %s by %s.%s</div>',
+        $when,
+        $handles[$content->getAuthorPHID()]->renderLink(),
+        $project_info);
 
 
       $doc_status = $document->getStatus();
@@ -145,12 +144,11 @@ final class PhrictionDocumentController
         throw new Exception("Unknown document status '{$doc_status}'!");
       }
 
-      $page_content =
-        '<div class="phriction-content">'.
-          $index_link.
-          $byline.
-          $core_content.
-          '</div>';
+      $page_content = hsprintf(
+        '<div class="phriction-content">%s%s%s</div>',
+        $index_link,
+        $byline,
+        $core_content);
     }
 
     if ($version_note) {
@@ -170,17 +168,14 @@ final class PhrictionDocumentController
     $header = id(new PhabricatorHeaderView())
       ->setHeader($page_title);
 
-    $page =
-      $crumbs->render().
-      $header->render().
-      $actions->render().
-      $version_note.
-      $page_content.
-      $children;
-
     return $this->buildApplicationPage(
       array(
-        $page,
+        $crumbs->render(),
+        $header->render(),
+        $actions->render(),
+        $version_note,
+        $page_content,
+        $children,
       ),
       array(
         'title'   => $page_title,
@@ -309,31 +304,32 @@ final class PhrictionDocumentController
     $children = isort($children, 'title');
 
     $list = array();
-    $list[] = '<ul>';
     foreach ($children as $child) {
+      $list[] = hsprintf('<li>');
       $list[] = $this->renderChildDocumentLink($child);
       $grand = idx($grandchildren, $child['slug'], array());
       if ($grand) {
-        $list[] = '<ul>';
+        $list[] = hsprintf('<ul>');
         foreach ($grand as $grandchild) {
+          $list[] = hsprintf('<li>');
           $list[] = $this->renderChildDocumentLink($grandchild);
+          $list[] = hsprintf('</li>');
         }
-        $list[] = '</ul>';
+        $list[] = hsprintf('</ul>');
       }
+      $list[] = hsprintf('</li>');
     }
     if ($more_children) {
-      $list[] = '<li>'.pht('More...').'</li>';
+      $list[] = phutil_tag('li', array(), pht('More...'));
     }
-    $list[] = '</ul>';
-    $list = implode("\n", $list);
 
-    return
+    return hsprintf(
       '<div class="phriction-children">'.
-        '<div class="phriction-children-header">'.
-          pht('Document Hierarchy').
-        '</div>'.
-        $list.
-      '</div>';
+        '<div class="phriction-children-header">%s</div>'.
+        '%s'.
+      '</div>',
+      pht('Document Hierarchy'),
+      phutil_tag('ul', array(), $list));
   }
 
   private function renderChildDocumentLink(array $info) {
@@ -346,10 +342,10 @@ final class PhrictionDocumentController
       $title);
 
     if (isset($info['empty'])) {
-      $item = '<em>'.$item.'</em>';
+      $item = phutil_tag('em', array(), $item);
     }
 
-    return '<li>'.$item.'</li>';
+    return $item;
   }
 
 }

src/applications/phriction/controller/PhrictionEditController.php

@@ -231,17 +231,15 @@ final class PhrictionEditController
       ->setHeader($panel_header)
       ->appendChild($form);
 
-    $preview_panel =
+    $preview_panel = hsprintf(
       '<div class="aphront-panel-preview aphront-panel-preview-wide">
-        <div class="phriction-document-preview-header">
-          '.pht('Document Preview').'
-        </div>
+        <div class="phriction-document-preview-header">%s</div>
         <div id="document-preview">
-          <div class="aphront-panel-preview-loading-text">
-            '.pht('Loading preview...').'
-          </div>
+          <div class="aphront-panel-preview-loading-text">%s</div>
         </div>
-      </div>';
+      </div>',
+      pht('Document Preview'),
+      pht('Loading preview...'));
 
     Javelin::initBehavior(
       'phriction-document-preview',

src/applications/ponder/controller/PonderQuestionAskController.php

@@ -74,14 +74,13 @@ final class PonderQuestionAskController extends PonderController {
         id(new AphrontFormSubmitControl())
         ->setValue('Ask Away!'));
 
-    $preview =
+    $preview = hsprintf(
       '<div class="aphront-panel-flush">'.
         '<div id="question-preview">'.
-          '<span class="aphront-panel-preview-loading-text">'.
-            pht('Loading question preview...').
-          '</span>'.
+          '<span class="aphront-panel-preview-loading-text">%s</span>'.
         '</div>'.
-      '</div>';
+      '</div>',
+      pht('Loading question preview...'));
 
     Javelin::initBehavior(
       'ponder-feedback-preview',

src/applications/search/controller/PhabricatorSearchController.php

@@ -242,18 +242,18 @@ final class PhabricatorSearchController
             ->setObject(idx($objects, $phid));
           $results[] = $view->render();
         }
-        $results =
+        $results = hsprintf(
           '<div class="phabricator-search-result-list">'.
-            implode("\n", $results).
-            '<div class="search-results-pager">'.
-              $pager->render().
-            '</div>'.
-          '</div>';
+            '%s'.
+            '<div class="search-results-pager">%s</div>'.
+          '</div>',
+          phutil_implode_html("\n", $results),
+          $pager->render());
       } else {
-        $results =
+        $results = hsprintf(
           '<div class="phabricator-search-result-list">'.
             '<p class="phabricator-search-no-results">No search results.</p>'.
-          '</div>';
+          '</div>');
       }
     } else {
       $results = null;

src/applications/uiexample/examples/PhabricatorActionListExample.php

@@ -104,7 +104,7 @@ final class PhabricatorActionListExample extends PhabricatorUIExample {
 
     return array(
       $view,
-      '<div style="clear: both;"></div>',
+      hsprintf('<div style="clear: both;"></div>'),
       $notices,
     );
   }

src/applications/uiexample/examples/PhabricatorButtonsExample.php

@@ -35,11 +35,11 @@ final class PhabricatorButtonsExample extends PhabricatorUIExample {
             ),
             ucwords($size.' '.$color.' '.$tag));
 
-          $view[] = '<br /><br />';
+          $view[] = hsprintf('<br /><br />');
         }
       }
     }
 
-    return '<div style="margin: 1em 2em;">'.implode('', $view).'</div>';
+    return phutil_tag('div', array('style' => 'margin: 1em 2em;'), $view);
   }
 }

src/applications/uiexample/examples/PhabricatorTagExample.php

@@ -19,7 +19,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setName('@alincoln')
       ->setHref('#');
     $tags[] = ' how is stuff?';
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
 
 
     $tags[] = 'Did you hear that ';
@@ -41,7 +41,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setDotColor(PhabricatorTagView::COLOR_GREY)
       ->setHref('#');
     $tags[] = ' is gone?';
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
 
     $tags[] = 'Take a look at ';
     $tags[] = id(new PhabricatorTagView())
@@ -49,7 +49,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setName('D123')
       ->setHref('#');
     $tags[] = ' when you get a chance.';
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
 
     $tags[] = 'Hmm? ';
     $tags[] = id(new PhabricatorTagView())
@@ -63,7 +63,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setBackgroundColor(PhabricatorTagView::COLOR_BLACK)
       ->setName('Abandoned');
     $tags[] = '.';
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
 
     $tags[] = 'I hope someone is going to ';
     $tags[] = id(new PhabricatorTagView())
@@ -77,7 +77,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setBackgroundColor(PhabricatorTagView::COLOR_REDORANGE)
       ->setName('High Priority');
     $tags[] = '!';
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
 
 
     $tags[] = id(new PhabricatorHeaderView())
@@ -89,7 +89,7 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
         ->setType(PhabricatorTagView::TYPE_STATE)
         ->setBackgroundColor($color)
         ->setName(ucwords($color));
-      $tags[] = '<br /><br />';
+      $tags[] = hsprintf('<br /><br />');
     }
 
     $tags[] = id(new PhabricatorHeaderView())
@@ -101,14 +101,14 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setDotColor(PhabricatorTagView::COLOR_RED)
       ->setBarColor(PhabricatorTagView::COLOR_RED)
       ->setName('Christmas');
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
     $tags[] = id(new PhabricatorTagView())
       ->setType(PhabricatorTagView::TYPE_OBJECT)
       ->setBackgroundColor(PhabricatorTagView::COLOR_ORANGE)
       ->setDotColor(PhabricatorTagView::COLOR_BLACK)
       ->setBarColor(PhabricatorTagView::COLOR_BLACK)
       ->setName('Halloween');
-    $tags[] = '<br /><br />';
+    $tags[] = hsprintf('<br /><br />');
     $tags[] = id(new PhabricatorTagView())
       ->setType(PhabricatorTagView::TYPE_STATE)
       ->setBackgroundColor(PhabricatorTagView::COLOR_MAGENTA)
@@ -116,10 +116,9 @@ final class PhabricatorTagExample extends PhabricatorUIExample {
       ->setBarColor(PhabricatorTagView::COLOR_BLUE)
       ->setName('Easter');
 
-    return array(
-      '<div style="padding: 1em 2em;">',
-      $tags,
-      '</div>',
-    );
+    return phutil_tag(
+      'div',
+      array('style' => 'padding: 1em 2em;'),
+      $tags);
   }
 }

src/applications/uiexample/examples/PhabricatorUINotificationExample.php

@@ -23,7 +23,7 @@ final class PhabricatorUINotificationExample extends PhabricatorUIExample {
       ),
       'Show Notification');
 
-    $content = '<div style="padding: 1em 3em;">'.$content.'</content>';
+    $content = hsprintf('<div style="padding: 1em 3em;">%s</div>', $content);
 
     return $content;
   }

src/view/layout/AphrontMiniPanelView.php

@@ -3,8 +3,9 @@
 final class AphrontMiniPanelView extends AphrontView {
 
   public function render() {
-    return hsprintf(
-      '<div class="aphront-mini-panel-view">%s</div>',
+    return phutil_tag(
+      'div',
+      array('class' => 'aphront-mini-panel-view'),
       $this->renderChildren());
   }
 

src/view/page/menu/PhabricatorMainMenuView.php

@@ -409,7 +409,9 @@ final class PhabricatorMainMenuView extends AphrontView {
       '');
 
     return array(
-      $bubble_tag.$message_tag, $notification_dropdown);
+      hsprintf('%s%s', $bubble_tag, $message_tag),
+      $notification_dropdown,
+    );
   }
 
   private function renderMenuIcon($name) {