From a1ff679f41f83a6d34a8e918d6d742867f0d8d55 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Jan 2013 17:07:07 -0800 Subject: [PATCH] Fix AphrontCrumbView (phutil_tag) Summary: Proper fix is to do some layout work in Diffusion. Short of that, make this escape properly. Test Plan: Viewed various crumbs, no more overescaping for non-diffusion crumbs. Reviewers: vrana Reviewed By: vrana CC: aran Differential Revision: https://secure.phabricator.com/D4641 --- .../controller/DiffusionController.php | 21 ++++++++++--------- src/view/layout/PhabricatorCrumbView.php | 19 ++++------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index 14b734f43d..cc30b41141 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -246,9 +246,9 @@ abstract class DiffusionController extends PhabricatorController { break; case 'change': $view_name = 'Change'; - $crumb_list[] = $crumb->setRawName( - phutil_escape_html($path).' ('.$commit_link.')' - ); + $crumb_list[] = $crumb->setName( + phutil_safe_html( + phutil_escape_html($path).' ('.$commit_link.')')); return $crumb_list; } @@ -293,7 +293,7 @@ abstract class DiffusionController extends PhabricatorController { $path_sections = '/'.implode('/', $path_sections); $crumb_list[] = id(new PhabricatorCrumbView()) - ->setRawName($path_sections); + ->setName(phutil_safe_html($path_sections)); } $last_crumb = array_pop($crumb_list); @@ -308,13 +308,14 @@ abstract class DiffusionController extends PhabricatorController { ) + $uri_params), ), 'Jump to HEAD'); - $last_crumb->setRawName( - $last_crumb->getNameForRender() . " @ {$commit_link} ({$jump_link})" - ); + + $name = $last_crumb->getName(); + $name = phutil_safe_html($name." @ {$commit_link} ({$jump_link})"); + $last_crumb->setName($name); } else if ($spec['view'] != 'lint') { - $last_crumb->setRawName( - $last_crumb->getNameForRender() . " @ HEAD" - ); + $name = $last_crumb->getName(); + $name = phutil_safe_html($name.' @ HEAD'); + $last_crumb->setName($name); } $crumb_list[] = $last_crumb; diff --git a/src/view/layout/PhabricatorCrumbView.php b/src/view/layout/PhabricatorCrumbView.php index 332d2f5465..11d311172b 100644 --- a/src/view/layout/PhabricatorCrumbView.php +++ b/src/view/layout/PhabricatorCrumbView.php @@ -6,25 +6,14 @@ final class PhabricatorCrumbView extends AphrontView { private $href; private $icon; private $isLastCrumb; - private $rawName; - - /** - * Allows for custom HTML inside the name field. - * - * NOTE: you must handle escaping user text if you use this method. - */ - public function setRawName($raw_name) { - $this->rawName = $raw_name; - return $this; - } public function setName($name) { $this->name = $name; return $this; } - public function getNameForRender() { - return nonempty($this->rawName, phutil_escape_html($this->name)); + public function getName() { + return $this->name; } public function setHref($href) { @@ -63,12 +52,12 @@ final class PhabricatorCrumbView extends AphrontView { ''); } - $name = phutil_render_tag( + $name = phutil_tag( 'span', array( 'class' => 'phabricator-crumb-name', ), - $this->getNameForRender()); + $this->name); $divider = null; if (!$this->isLastCrumb) {