Skip to content

Commit b5fde42

Browse files
author
Oleksandr Gorkun
committed
MAGETWO-55858: Output escaping methods shouldn't be part of AbstractBlock
1 parent 148d6cc commit b5fde42

File tree

5 files changed

+117
-17
lines changed

5 files changed

+117
-17
lines changed

‎app/code/Magento/Catalog/view/frontend/templates/product/list.phtml‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,16 @@ use Magento\Framework\App\Action\Action;
1313
* Product list template
1414
*
1515
* @var $block \Magento\Catalog\Block\Product\ListProduct
16+
* @var \Magento\Framework\Escaper $escaper
1617
*/
1718
?>
1819
<?php
1920
$_productCollection = $block->getLoadedProductCollection();
21+
/** @var \Magento\Catalog\Helper\Output $_helper */
2022
$_helper = $this->helper(Magento\Catalog\Helper\Output::class);
2123
?>
2224
<?php if (!$_productCollection->count()) :?>
23-
<div class="message info empty"><div><?= $block->escapeHtml(__('We can\'t find products matching the selection.')) ?></div></div>
25+
<div class="message info empty"><div><?= $escaper->escapeHtml(__('We can\'t find products matching the selection.')) ?></div></div>
2426
<?php else :?>
2527
<?= $block->getToolbarHtml() ?>
2628
<?= $block->getAdditionalHtml() ?>
@@ -55,7 +57,7 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
5557
}
5658
?>
5759
<?php // Product Image ?>
58-
<a href="<?= $block->escapeUrl($_product->getProductUrl()) ?>"
60+
<a href="<?= $escaper->escapeUrl($_product->getProductUrl()) ?>"
5961
class="product photo product-item-photo"
6062
tabindex="-1">
6163
<?= $productImage->toHtml() ?>
@@ -66,7 +68,7 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
6668
?>
6769
<strong class="product name product-item-name">
6870
<a class="product-item-link"
69-
href="<?= $block->escapeUrl($_product->getProductUrl()) ?>">
71+
href="<?= $escaper->escapeUrl($_product->getProductUrl()) ?>">
7072
<?= /* @noEscape */ $_helper->productAttribute($_product, $_product->getName(), 'name') ?>
7173
</a>
7274
</strong>
@@ -77,13 +79,13 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
7779
<?php endif; ?>
7880

7981
<div class="product-item-inner">
80-
<div class="product actions product-item-actions"<?= strpos($pos, $viewMode . '-actions') ? $block->escapeHtmlAttr($position) : '' ?>>
81-
<div class="actions-primary"<?= strpos($pos, $viewMode . '-primary') ? $block->escapeHtmlAttr($position) : '' ?>>
82+
<div class="product actions product-item-actions"<?= strpos($pos, $viewMode . '-actions') ? $escaper->escapeHtmlAttr($position) : '' ?>>
83+
<div class="actions-primary"<?= strpos($pos, $viewMode . '-primary') ? $escaper->escapeHtmlAttr($position) : '' ?>>
8284
<?php if ($_product->isSaleable()) :?>
8385
<?php $postParams = $block->getAddToCartPostParams($_product); ?>
8486
<form data-role="tocart-form"
85-
data-product-sku="<?= $block->escapeHtml($_product->getSku()) ?>"
86-
action="<?= $block->escapeUrl($postParams['action']) ?>"
87+
data-product-sku="<?= $escaper->escapeHtml($_product->getSku()) ?>"
88+
action="<?= $escaper->escapeUrl($postParams['action']) ?>"
8789
method="post">
8890
<input type="hidden"
8991
name="product"
@@ -92,20 +94,20 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
9294
value="<?= /* @noEscape */ $postParams['data'][Action::PARAM_NAME_URL_ENCODED] ?>">
9395
<?= $block->getBlockHtml('formkey') ?>
9496
<button type="submit"
95-
title="<?= $block->escapeHtmlAttr(__('Add to Cart')) ?>"
97+
title="<?= $escaper->escapeHtmlAttr(__('Add to Cart')) ?>"
9698
class="action tocart primary">
97-
<span><?= $block->escapeHtml(__('Add to Cart')) ?></span>
99+
<span><?= $escaper->escapeHtml(__('Add to Cart')) ?></span>
98100
</button>
99101
</form>
100102
<?php else :?>
101103
<?php if ($_product->isAvailable()) :?>
102-
<div class="stock available"><span><?= $block->escapeHtml(__('In stock')) ?></span></div>
104+
<div class="stock available"><span><?= $escaper->escapeHtml(__('In stock')) ?></span></div>
103105
<?php else :?>
104-
<div class="stock unavailable"><span><?= $block->escapeHtml(__('Out of stock')) ?></span></div>
106+
<div class="stock unavailable"><span><?= $escaper->escapeHtml(__('Out of stock')) ?></span></div>
105107
<?php endif; ?>
106108
<?php endif; ?>
107109
</div>
108-
<div data-role="add-to-links" class="actions-secondary"<?= strpos($pos, $viewMode . '-secondary') ? $block->escapeHtmlAttr($position) : '' ?>>
110+
<div data-role="add-to-links" class="actions-secondary"<?= strpos($pos, $viewMode . '-secondary') ? $escaper->escapeHtmlAttr($position) : '' ?>>
109111
<?php if ($addToBlock = $block->getChildBlock('addto')) :?>
110112
<?= $addToBlock->setProduct($_product)->getChildHtml() ?>
111113
<?php endif; ?>
@@ -114,9 +116,9 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
114116
<?php if ($showDescription) :?>
115117
<div class="product description product-item-description">
116118
<?= /* @noEscape */ $_helper->productAttribute($_product, $_product->getShortDescription(), 'short_description') ?>
117-
<a href="<?= $block->escapeUrl($_product->getProductUrl()) ?>"
119+
<a href="<?= $escaper->escapeUrl($_product->getProductUrl()) ?>"
118120
title="<?= /* @noEscape */ $_productNameStripped ?>"
119-
class="action more"><?= $block->escapeHtml(__('Learn More')) ?></a>
121+
class="action more"><?= $escaper->escapeHtml(__('Learn More')) ?></a>
120122
</div>
121123
<?php endif; ?>
122124
</div>
@@ -132,7 +134,7 @@ $_helper = $this->helper(Magento\Catalog\Helper\Output::class);
132134
{
133135
"[data-role=tocart-form], .form.map.checkout": {
134136
"catalogAddToCart": {
135-
"product_sku": "<?= $block->escapeJs($_product->getSku()) ?>"
137+
"product_sku": "<?= $escaper->escapeJs($_product->getSku()) ?>"
136138
}
137139
}
138140
}

‎dev/tests/integration/testsuite/Magento/Framework/View/TemplateEngine/PhpTest.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Framework\View\TemplateEngine;
10+
11+
use Magento\Framework\View\Element\BlockInterface;
12+
use Magento\TestFramework\Helper\Bootstrap;
13+
use PHPUnit\Framework\TestCase;
14+
15+
/**
16+
* Testing .phtml templating.
17+
*/
18+
class PhpTest extends TestCase
19+
{
20+
/**
21+
* @var Php
22+
*/
23+
private $templateEngine;
24+
25+
/**
26+
* @inheritdoc
27+
*/
28+
protected function setUp()
29+
{
30+
$objectManager = Bootstrap::getObjectManager();
31+
$this->templateEngine = $objectManager->get(Php::class);
32+
}
33+
34+
/**
35+
* See that templates get access to certain variables.
36+
*
37+
* @return void
38+
*/
39+
public function testVariablesAvailable(): void
40+
{
41+
$block = new class implements BlockInterface {
42+
/**
43+
* @inheritDoc
44+
*/
45+
public function toHtml()
46+
{
47+
return '<b>BLOCK</b>';
48+
}
49+
};
50+
51+
$rendered = $this->templateEngine->render($block, __DIR__ .'/../_files/test_template.phtml');
52+
$this->assertEquals(
53+
'<p>This template has access to &lt;b&gt;$escaper&lt;/b&gt; and $block &quot;<b>BLOCK</b>&quot;</p>'
54+
.PHP_EOL,
55+
$rendered
56+
);
57+
}
58+
}

‎dev/tests/integration/testsuite/Magento/Framework/View/_files/test_template.phtml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
?>
7+
<?php
8+
// phpcs:disable
9+
?>
10+
<?php
11+
/**
12+
* Template meant for testing.
13+
*
14+
* @var \Magento\Framework\View\Element\BlockInterface $block
15+
* @var \Magento\Framework\Escaper $escaper
16+
*/
17+
?>
18+
<p>This template has access to <?= $escaper->escapeHtml('<b>$escaper</b>') ?> and $block &quot;<?= $block->toHtml() ?>&quot;</p>

‎lib/internal/Magento/Framework/View/Element/AbstractBlock.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
67

78
namespace Magento\Framework\View\Element;
89

@@ -887,6 +888,7 @@ public static function extractModuleName($className)
887888
* @param string|array $data
888889
* @param array|null $allowedTags
889890
* @return string
891+
* @deprecated Use $escaper directly in templates and in blocks.
890892
*/
891893
public function escapeHtml($data, $allowedTags = null)
892894
{
@@ -899,6 +901,7 @@ public function escapeHtml($data, $allowedTags = null)
899901
* @param string $string
900902
* @return string
901903
* @since 100.2.0
904+
* @deprecated Use $escaper directly in templates and in blocks.
902905
*/
903906
public function escapeJs($string)
904907
{
@@ -912,6 +915,7 @@ public function escapeJs($string)
912915
* @param boolean $escapeSingleQuote
913916
* @return string
914917
* @since 100.2.0
918+
* @deprecated Use $escaper directly in templates and in blocks.
915919
*/
916920
public function escapeHtmlAttr($string, $escapeSingleQuote = true)
917921
{
@@ -924,6 +928,7 @@ public function escapeHtmlAttr($string, $escapeSingleQuote = true)
924928
* @param string $string
925929
* @return string
926930
* @since 100.2.0
931+
* @deprecated Use $escaper directly in templates and in blocks.
927932
*/
928933
public function escapeCss($string)
929934
{
@@ -951,6 +956,7 @@ public function stripTags($data, $allowableTags = null, $allowHtmlEntities = fal
951956
*
952957
* @param string $string
953958
* @return string
959+
* @deprecated Use $escaper directly in templates and in blocks.
954960
*/
955961
public function escapeUrl($string)
956962
{

‎lib/internal/Magento/Framework/View/TemplateEngine/Php.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Framework\View\TemplateEngine;
79

10+
use Magento\Framework\Escaper;
811
use Magento\Framework\View\Element\BlockInterface;
912
use Magento\Framework\View\TemplateEngineInterface;
1013

@@ -27,14 +30,23 @@ class Php implements TemplateEngineInterface
2730
*/
2831
protected $_helperFactory;
2932

33+
/**
34+
* @var Escaper
35+
*/
36+
private $escaper;
37+
3038
/**
3139
* Constructor
3240
*
3341
* @param \Magento\Framework\ObjectManagerInterface $helperFactory
42+
* @param Escaper|null $escaper
3443
*/
35-
public function __construct(\Magento\Framework\ObjectManagerInterface $helperFactory)
36-
{
44+
public function __construct(
45+
\Magento\Framework\ObjectManagerInterface $helperFactory,
46+
?Escaper $escaper = null
47+
) {
3748
$this->_helperFactory = $helperFactory;
49+
$this->escaper = $escaper ?? $helperFactory->get(Escaper::class);
3850
}
3951

4052
/**
@@ -48,6 +60,7 @@ public function __construct(\Magento\Framework\ObjectManagerInterface $helperFac
4860
* @param array $dictionary
4961
* @return string
5062
* @throws \Exception
63+
* @SuppressWarnings(PHPMD.UnusedLocalVariable)
5164
*/
5265
public function render(BlockInterface $block, $fileName, array $dictionary = [])
5366
{
@@ -56,6 +69,9 @@ public function render(BlockInterface $block, $fileName, array $dictionary = [])
5669
$tmpBlock = $this->_currentBlock;
5770
$this->_currentBlock = $block;
5871
extract($dictionary, EXTR_SKIP);
72+
//So it can be used in the template.
73+
$escaper = $this->escaper;
74+
// phpcs:ignore
5975
include $fileName;
6076
$this->_currentBlock = $tmpBlock;
6177
} catch (\Exception $exception) {

0 commit comments

Comments
 (0)