From 9861cafd14fd6e54cc3a18a258b5f6c84300d316 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Mon, 28 May 2012 23:29:05 +0200 Subject: [PATCH 1/8] updated ZendTest\View\Helper\CurrencyTest to use the simplified cache API --- test/Helper/CurrencyTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Helper/CurrencyTest.php b/test/Helper/CurrencyTest.php index 904a42c2..bfc2eddd 100644 --- a/test/Helper/CurrencyTest.php +++ b/test/Helper/CurrencyTest.php @@ -63,8 +63,8 @@ public function setUp() { $this->clearRegistry(); - $this->_cache = CacheFactory::adapterFactory('memory', array('memory_limit' => 0)); - Currency\Currency::setCache($this->_cache); + $cache = CacheFactory::adapterFactory('memory', array('memory_limit' => 0)); + Currency\Currency::setCache($cache); $this->helper = new Helper\Currency('de_AT'); } @@ -78,7 +78,6 @@ public function setUp() public function tearDown() { unset($this->helper); - $this->_cache->clear(CacheAdapter::MATCH_ALL); $this->clearRegistry(); } From aded512b0fa3e8fec0e0ae15ce1ff834157aef70 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 2 Jun 2012 10:59:17 +0200 Subject: [PATCH 2/8] Fixing typo, adding method docbloc, returning early instead of using 'else if' --- src/Helper/Navigation/AbstractHelper.php | 44 +++++++++++++++--------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/Helper/Navigation/AbstractHelper.php b/src/Helper/Navigation/AbstractHelper.php index 83f3f6f4..bacd3175 100644 --- a/src/Helper/Navigation/AbstractHelper.php +++ b/src/Helper/Navigation/AbstractHelper.php @@ -192,19 +192,31 @@ public function getContainer() return $this->container; } + /** + * Verifies container and eventually fetches it from service locator if it is a string + * + * @param \Zend\Navigation\AbstractContainer|string|null $container + * @throws \Zend\View\Exception\InvalidArgumentException + */ protected function parseContainer(&$container = null) { if (null === $container) { - ; // intentionally left blank - } else if (is_string($container)) { + return; + } + + if (is_string($container)) { if (!$this->getServiceLocator()) { throw new Exception\InvalidArgumentException(sprintf( - 'Attempted to set container with alias "%s" but no ServiceLocator wwas set', + 'Attempted to set container with alias "%s" but no ServiceLocator was set', $container )); } + $container = $this->getServiceLocator()->get($container); - } else if (!$container instanceof Navigation\AbstractContainer) { + return; + } + + if (!$container instanceof Navigation\AbstractContainer) { throw new Exception\InvalidArgumentException( 'Container must be a string alias or an instance of ' . 'Zend\Navigation\AbstractContainer' @@ -215,7 +227,7 @@ protected function parseContainer(&$container = null) /** * Sets the minimum depth a page must have to be included when rendering * - * @param int $minDepth [optional] minimum depth. Default is null, which + * @param int $minDepth [optional] minimum depth. Default is null, which * sets no minimum depth. * @return AbstractHelper fluent interface, returns self */ @@ -245,7 +257,7 @@ public function getMinDepth() /** * Sets the maximum depth a page can have to be included when rendering * - * @param int $maxDepth [optional] maximum depth. Default is null, which + * @param int $maxDepth [optional] maximum depth. Default is null, which * sets no maximum depth. * @return AbstractHelper fluent interface, returns self */ @@ -297,9 +309,9 @@ public function getIndent() * * Implements {@link HelperInterface::setTranslator()}. * - * @param mixed $translator [optional] translator. Expects an object of + * @param mixed $translator [optional] translator. Expects an object of * type {@link Translator\Adapter\AbstractAdapter} - * or {@link Translator\Translator}, or null. + * or {@link Translator\Translator}, or null. * Default is null, which sets no translator. * @return AbstractHelper fluent interface, returns self */ @@ -363,7 +375,7 @@ public function getAcl() * * Implements {@link HelperInterface::setRole()}. * - * @param mixed $role [optional] role to set. Expects a string, an + * @param mixed $role [optional] role to set. Expects a string, an * instance of type {@link Acl\Role\RoleInterface}, or null. Default * is null, which will set no role. * @return AbstractHelper fluent interface, returns self @@ -377,7 +389,7 @@ public function setRole($role = null) $this->role = $role; } else { throw new Exception\InvalidArgumentException(sprintf( - '$role must be a string, null, or an instance of ' + '$role must be a string, null, or an instance of ' . 'Zend\Acl\Role\RoleInterface; %s given', (is_object($role) ? get_class($role) : gettype($role)) )); @@ -456,7 +468,7 @@ public function setRenderInvisible($renderInvisible = true) * * Implements {@link HelperInterface::setUseTranslator()}. * - * @param bool $useTranslator [optional] whether translator should be used. + * @param bool $useTranslator [optional] whether translator should be used. * Default is true. * @return AbstractHelper fluent interface, returns self */ @@ -691,8 +703,8 @@ public function htmlify(AbstractPage $page) * will not be accepted if it is the descendant of a non-accepted page. * * @param AbstractPage $page page to check - * @param bool $recursive [optional] if true, page will not be - * accepted if it is the descendant of a + * @param bool $recursive [optional] if true, page will not be + * accepted if it is the descendant of a * page that is not accepted. Default is true. * @return bool whether page should be accepted */ @@ -822,7 +834,7 @@ public static function setDefaultAcl(Acl\Acl $acl = null) * Sets default ACL role(s) to use when iterating pages if not explicitly * set later with {@link setRole()} * - * @param mixed $role [optional] role to set. Expects null, string, or an + * @param mixed $role [optional] role to set. Expects null, string, or an * instance of {@link Acl\Role\RoleInterface}. Default is null, which * sets no default role. * @return void @@ -830,8 +842,8 @@ public static function setDefaultAcl(Acl\Acl $acl = null) */ public static function setDefaultRole($role = null) { - if (null === $role - || is_string($role) + if (null === $role + || is_string($role) || $role instanceof Acl\Role\RoleInterface ) { self::$defaultRole = $role; From cdef0ea55964dcb43d22b54d9b18be2e7101f281 Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Fri, 8 Jun 2012 15:41:51 +0200 Subject: [PATCH 3/8] block doc update --- src/Helper/HeadScript.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index 6ef90750..f013d8a2 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -55,7 +55,7 @@ class HeadScript extends Placeholder\Container\Standalone protected $_arbitraryAttributes = false; /** - * Are arbitrary attributes allowed? + * Should scripts be escaped with cdata or html comment tags? * @var bool */ protected $_escapeScript = true; From 062df52c82dff45d7d137160feff199817ee24bd Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Fri, 8 Jun 2012 16:56:33 +0200 Subject: [PATCH 4/8] refactored double ternary use --- src/Helper/HeadScript.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index f013d8a2..065b0f19 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -494,8 +494,12 @@ public function toString($indent = null) } else { $useCdata = $this->useCdata ? true : false; } - $escapeStart = $this->escapeScript() ? ($useCdata) ? '//escapeScript() ? ($useCdata) ? '//]]>' : '//-->' : ''; + + $escapeStart = $escapeEnd = ''; + if ($this->escapeScript()) { + $escapeStart = ($useCdata) ? '//' : '//-->'; + } $items = array(); $this->getContainer()->ksort(); From 48742fc844aa9cfc9ba4c9dac2e466dbe6f97d77 Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Tue, 12 Jun 2012 11:02:55 +0200 Subject: [PATCH 5/8] - modifies the way the feature works, not 'globally' like allowed-arbitrary-attributes but by passing an (noescape) attribute value on each invokation - note that this commit also makes sure setAllowArbitraryAttributes(true) does not render the 'conditional' and 'noescape' tag. - adds unittest. --- src/Helper/HeadScript.php | 64 ++++++++++------------------------ test/Helper/HeadScriptTest.php | 38 +++++++++++++++++++- 2 files changed, 55 insertions(+), 47 deletions(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index 065b0f19..2cfa04d7 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -54,12 +54,6 @@ class HeadScript extends Placeholder\Container\Standalone */ protected $_arbitraryAttributes = false; - /** - * Should scripts be escaped with cdata or html comment tags? - * @var bool - */ - protected $_escapeScript = true; - /**#@+ * Capture type and/or attributes (used for hinting during capture) * @var string @@ -400,38 +394,6 @@ public function arbitraryAttributesAllowed() return $this->_arbitraryAttributes; } - /** - * Set flag indicating if scripts should be escaped with one of these - * - * // - * - * @param bool $flag Set flag - * @return \Zend\View\Helper\HeadScript - */ - public function setEscapeScript($flag) - { - $this->_escapeScript = (bool) $flag; - return $this; - } - - /** - * Are arbitrary attributes allowed? - * - * @return bool - */ - public function escapeScript() - { - return $this->_escapeScript; - } - /** * Create script HTML * @@ -446,8 +408,8 @@ public function itemToString($item, $indent, $escapeStart, $escapeEnd) $attrString = ''; if (!empty($item->attributes)) { foreach ($item->attributes as $key => $value) { - if (!$this->arbitraryAttributesAllowed() - && !in_array($key, $this->_optionalAttributes)) + if ((!$this->arbitraryAttributesAllowed() + && !in_array($key, $this->_optionalAttributes)) || in_array($key, array('conditional', 'noescape'))) { continue; } @@ -458,10 +420,23 @@ public function itemToString($item, $indent, $escapeStart, $escapeEnd) } } + + $addScriptEscape = !(isset($item->attributes['noescape']) && filter_var($item->attributes['noescape'], FILTER_VALIDATE_BOOLEAN)); + $type = ($this->_autoEscape) ? $this->_escape($item->type) : $item->type; $html = ''; @@ -495,11 +470,8 @@ public function toString($indent = null) $useCdata = $this->useCdata ? true : false; } - $escapeStart = $escapeEnd = ''; - if ($this->escapeScript()) { - $escapeStart = ($useCdata) ? '//' : '//-->'; - } + $escapeStart = ($useCdata) ? '//' : '//-->'; $items = array(); $this->getContainer()->ksort(); diff --git a/test/Helper/HeadScriptTest.php b/test/Helper/HeadScriptTest.php index 60c97d7b..2080b3ed 100644 --- a/test/Helper/HeadScriptTest.php +++ b/test/Helper/HeadScriptTest.php @@ -430,5 +430,41 @@ public function testContainerMaintainsCorrectOrderOfItems() $this->assertEquals($expected, $test); } -} + public function testConditionalWithAllowArbitraryAttributesDoesNotIncludeConditionalScript() + { + $this->helper->__invoke()->setAllowArbitraryAttributes(true); + $this->helper->__invoke()->appendFile('/js/foo.js', 'text/javascript', array('conditional' => 'lt IE 7')); + $test = $this->helper->__invoke()->toString(); + + $this->assertNotContains('conditional', $test); + } + + public function testNoEscapeWithAllowArbitraryAttributesDoesNotIncludeNoEscapeScript() + { + $this->helper->__invoke()->setAllowArbitraryAttributes(true); + $this->helper->__invoke()->appendScript('// some script', 'text/javascript', array('noescape' => true)); + $test = $this->helper->__invoke()->toString(); + + $this->assertNotContains('noescape', $test); + } + + public function testNoEscapeDefaultsToFalse() + { + $this->helper->__invoke()->appendScript('// some script' . PHP_EOL, 'text/javascript', array()); + $test = $this->helper->__invoke()->toString(); + + $this->assertContains('//', $test); + } + + public function testNoEscapeTrue() + { + $this->helper->__invoke()->appendScript('// some script' . PHP_EOL, 'text/javascript', array('noescape' => true)); + $test = $this->helper->__invoke()->toString(); + + $this->assertNotContains('//', $test); + } + +} From eb4a80484ddc9dfcae7d7e535536521fc5225f9c Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Tue, 12 Jun 2012 11:29:45 +0200 Subject: [PATCH 6/8] curly braces --- src/Helper/HeadScript.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index 2cfa04d7..c312fe54 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -428,14 +428,16 @@ public function itemToString($item, $indent, $escapeStart, $escapeEnd) if (!empty($item->source)) { $html .= PHP_EOL ; - if ($addScriptEscape) + if ($addScriptEscape) { $html .= $indent . ' ' . $escapeStart . PHP_EOL; + } $html .= $indent . ' ' . $item->source; - if ($addScriptEscape) + if ($addScriptEscape) { $html .= $indent . ' ' . $escapeEnd . PHP_EOL; - + } + $html .= $indent; } $html .= ''; From 4a7ad28a777931c7960dc5c010edf84e24e25720 Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Tue, 12 Jun 2012 17:52:38 +0200 Subject: [PATCH 7/8] || on it's own line --- src/Helper/HeadScript.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index c312fe54..08526085 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -409,7 +409,8 @@ public function itemToString($item, $indent, $escapeStart, $escapeEnd) if (!empty($item->attributes)) { foreach ($item->attributes as $key => $value) { if ((!$this->arbitraryAttributesAllowed() - && !in_array($key, $this->_optionalAttributes)) || in_array($key, array('conditional', 'noescape'))) + && !in_array($key, $this->_optionalAttributes)) + || in_array($key, array('conditional', 'noescape'))) { continue; } From a0007957df36e29f8a55f7543bb995c58f290025 Mon Sep 17 00:00:00 2001 From: Bas Kamer Date: Tue, 12 Jun 2012 20:46:49 +0200 Subject: [PATCH 8/8] prefered syntax change --- src/Helper/HeadScript.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Helper/HeadScript.php b/src/Helper/HeadScript.php index 08526085..45cbeb64 100644 --- a/src/Helper/HeadScript.php +++ b/src/Helper/HeadScript.php @@ -408,8 +408,7 @@ public function itemToString($item, $indent, $escapeStart, $escapeEnd) $attrString = ''; if (!empty($item->attributes)) { foreach ($item->attributes as $key => $value) { - if ((!$this->arbitraryAttributesAllowed() - && !in_array($key, $this->_optionalAttributes)) + if ((!$this->arbitraryAttributesAllowed() && !in_array($key, $this->_optionalAttributes)) || in_array($key, array('conditional', 'noescape'))) { continue;