Skip to content

Commit 941fa8c

Browse files
author
Stanislav Idolov
authored
ENGCOM-2891: [Forwardport] CSS load order incorrect using default_head_blocks.xml #1821 #17829 
2 parents fb415d7 + 9d9c9b9 commit 941fa8c

File tree

6 files changed

+101
-34
lines changed

6 files changed

+101
-34
lines changed

‎lib/internal/Magento/Framework/View/Layout/etc/head.xsd‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<xs:attribute name="sizes" type="xs:string"/>
1919
<xs:attribute name="target" type="xs:string"/>
2020
<xs:attribute name="type" type="xs:string"/>
21+
<xs:attribute name="order" type="xs:integer"/>
2122
<xs:attribute name="src_type" type="xs:string"/>
2223
</xs:complexType>
2324

‎lib/internal/Magento/Framework/View/Page/Config/Generator/Head.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class Head implements Layout\GeneratorInterface
4242
*/
4343
protected $assetProperties = [
4444
'ie_condition',
45+
'order'
4546
];
4647

4748
/**

‎lib/internal/Magento/Framework/View/Page/Config/Reader/Head.php‎

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use Magento\Framework\View\Layout;
99
use Magento\Framework\View\Page\Config as PageConfig;
10+
use Magento\Framework\View\Page\Config\Structure;
1011

1112
/**
1213
* Head structure reader is intended for collecting assets, title and metadata
@@ -71,38 +72,19 @@ public function interpret(
7172
Layout\Element $headElement
7273
) {
7374
$pageConfigStructure = $readerContext->getPageConfigStructure();
74-
/** @var \Magento\Framework\View\Layout\Element $node */
75+
76+
$orderedNodes = [];
77+
7578
foreach ($headElement as $node) {
76-
switch ($node->getName()) {
77-
case self::HEAD_CSS:
78-
case self::HEAD_SCRIPT:
79-
case self::HEAD_LINK:
80-
$this->addContentTypeByNodeName($node);
81-
$pageConfigStructure->addAssets($node->getAttribute('src'), $this->getAttributes($node));
82-
break;
83-
84-
case self::HEAD_REMOVE:
85-
$pageConfigStructure->removeAssets($node->getAttribute('src'));
86-
break;
87-
88-
case self::HEAD_TITLE:
89-
$pageConfigStructure->setTitle(new \Magento\Framework\Phrase($node));
90-
break;
91-
92-
case self::HEAD_META:
93-
$this->setMetadata($pageConfigStructure, $node);
94-
break;
95-
96-
case self::HEAD_ATTRIBUTE:
97-
$pageConfigStructure->setElementAttribute(
98-
PageConfig::ELEMENT_TYPE_HEAD,
99-
$node->getAttribute('name'),
100-
$node->getAttribute('value')
101-
);
102-
break;
103-
104-
default:
105-
break;
79+
$nodeOrder = $node->getAttribute('order') ?: 0;
80+
$orderedNodes[$nodeOrder][] = $node;
81+
}
82+
83+
ksort($orderedNodes);
84+
foreach ($orderedNodes as $nodes) {
85+
/** @var \Magento\Framework\View\Layout\Element $node */
86+
foreach ($nodes as $node) {
87+
$this->processNode($node, $pageConfigStructure);
10688
}
10789
}
10890
return $this;
@@ -140,4 +122,46 @@ private function setMetadata($pageConfigStructure, $node)
140122

141123
$pageConfigStructure->setMetadata($metadataName, $node->getAttribute('content'));
142124
}
125+
126+
/**
127+
* Process given node based on it's name.
128+
*
129+
* @param Layout\Element $node
130+
* @param Structure $pageConfigStructure
131+
* @return void
132+
*/
133+
private function processNode(Layout\Element $node, Structure $pageConfigStructure)
134+
{
135+
switch ($node->getName()) {
136+
case self::HEAD_CSS:
137+
case self::HEAD_SCRIPT:
138+
case self::HEAD_LINK:
139+
$this->addContentTypeByNodeName($node);
140+
$pageConfigStructure->addAssets($node->getAttribute('src'), $this->getAttributes($node));
141+
break;
142+
143+
case self::HEAD_REMOVE:
144+
$pageConfigStructure->removeAssets($node->getAttribute('src'));
145+
break;
146+
147+
case self::HEAD_TITLE:
148+
$pageConfigStructure->setTitle(new \Magento\Framework\Phrase($node));
149+
break;
150+
151+
case self::HEAD_META:
152+
$this->setMetadata($pageConfigStructure, $node);
153+
break;
154+
155+
case self::HEAD_ATTRIBUTE:
156+
$pageConfigStructure->setElementAttribute(
157+
PageConfig::ELEMENT_TYPE_HEAD,
158+
$node->getAttribute('name'),
159+
$node->getAttribute('value')
160+
);
161+
break;
162+
163+
default:
164+
break;
165+
}
166+
}
143167
}

‎lib/internal/Magento/Framework/View/Test/Unit/Page/Config/Generator/HeadTest.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ protected function setUp()
6060
);
6161
}
6262

63+
/**
64+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
65+
*/
6366
public function testProcess()
6467
{
6568
$generatorContextMock = $this->createMock(Context::class);
@@ -82,6 +85,20 @@ public function testProcess()
8285
'content_type' => 'css',
8386
'media' => 'all',
8487
],
88+
'remoteCssOrderedLast' => [
89+
'src' => 'file-url-css-last',
90+
'src_type' => 'url',
91+
'content_type' => 'css',
92+
'media' => 'all',
93+
'order' => 30,
94+
],
95+
'remoteCssOrderedFirst' => [
96+
'src' => 'file-url-css-first',
97+
'src_type' => 'url',
98+
'content_type' => 'css',
99+
'media' => 'all',
100+
'order' => 10,
101+
],
85102
'remoteLink' => [
86103
'src' => 'file-url-link',
87104
'src_type' => 'url',
@@ -106,8 +123,14 @@ public function testProcess()
106123
->with('file-url-css', 'css', ['attributes' => ['media' => 'all']]);
107124
$this->pageConfigMock->expects($this->at(1))
108125
->method('addRemotePageAsset')
109-
->with('file-url-link', Head::VIRTUAL_CONTENT_TYPE_LINK, ['attributes' => ['media' => 'all']]);
126+
->with('file-url-css-last', 'css', ['attributes' => ['media' => 'all' ] , 'order' => 30]);
110127
$this->pageConfigMock->expects($this->at(2))
128+
->method('addRemotePageAsset')
129+
->with('file-url-css-first', 'css', ['attributes' => ['media' => 'all'] , 'order' => 10]);
130+
$this->pageConfigMock->expects($this->at(3))
131+
->method('addRemotePageAsset')
132+
->with('file-url-link', Head::VIRTUAL_CONTENT_TYPE_LINK, ['attributes' => ['media' => 'all']]);
133+
$this->pageConfigMock->expects($this->at(4))
111134
->method('addRemotePageAsset')
112135
->with('http://magento.dev/customcss/render/css', 'css', ['attributes' => ['media' => 'all']]);
113136
$this->pageConfigMock->expects($this->once())

‎lib/internal/Magento/Framework/View/Test/Unit/Page/Config/Reader/HeadTest.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function testInterpret()
5959

6060
$structureMock->expects($this->at(4))
6161
->method('addAssets')
62-
->with('path/file.css', ['src' => 'path/file.css', 'media' => 'all', 'content_type' => 'css'])
62+
->with('path/file-3.css', ['src' => 'path/file-3.css', 'media' => 'all', 'content_type' => 'css'])
6363
->willReturnSelf();
6464

6565
$structureMock->expects($this->at(5))
@@ -82,6 +82,22 @@ public function testInterpret()
8282
->with(Config::ELEMENT_TYPE_HEAD, 'head_attribute_name', 'head_attribute_value')
8383
->willReturnSelf();
8484

85+
$structureMock->expects($this->at(9))
86+
->method('addAssets')
87+
->with(
88+
'path/file-1.css',
89+
['src' => 'path/file-1.css', 'media' => 'all', 'content_type' => 'css', 'order' => 10]
90+
)
91+
->willReturnSelf();
92+
93+
$structureMock->expects($this->at(10))
94+
->method('addAssets')
95+
->with(
96+
'path/file-2.css',
97+
['src' => 'path/file-2.css', 'media' => 'all', 'content_type' => 'css', 'order' => 30]
98+
)
99+
->willReturnSelf();
100+
85101
$this->assertEquals($this->model, $this->model->interpret($readerContextMock, $element->children()[0]));
86102
}
87103
}

‎lib/internal/Magento/Framework/View/Test/Unit/Page/Config/_files/template_head.xml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
<meta name="meta_name" content="meta_content"/>
1212
<meta property="og:video:secure_url" content="https://secure.example.com/movie.swf" />
1313
<meta property="og:locale:alternate" content="uk_UA" />
14-
<css src="path/file.css" media="all" />
14+
<css src="path/file-1.css" order="10" media="all" />
15+
<css src="path/file-2.css" order="30" media="all" />
16+
<css src="path/file-3.css" media="all" />
1517
<script src="path/file.js" defer="defer"/>
1618
<link src="http://url.com" src_type="url"/>
1719
<remove src="path/remove/file.css"/>

0 commit comments

Comments
 (0)