Bug #91769
closed
The `f:asset.css` ViewHelper doesn't handle `priority=false` like in the documentation
Added by Julian Mair almost 4 years ago. Updated over 2 years ago.
100%
Description
Both of the new `asset` ViewHelpers can set the priority to either true or false, which means (according to the doc) to insert the tags either in <head> or directly before the closing <body> tag.
See here: https://docs.typo3.org/other/typo3/view-helper-reference/10.4/en-us/typo3/fluid/latest/Asset/Css.html#priority
But for the `f:asset.css` ViewHelper it's always in the <head> tag.
I narrowed it down to the pageRendererTemplateFile "EXT:frontend/Resources/Private/Templates/MainPage.html"... For the time being, I fixed it myself by overriding and restructuring the marker positions.
But the question is: is this a bug by Typo3 or just a wrong statement in the docs?
Updated by Jonas Eberle almost 4 years ago
Could you give an example and say how you would expect it to be put into html?
The important question is if you mean inline styles or not. `<style>` can only be in `<head>` to my knowledge while `<link>` may appear everywhere.
So we could decide if the implementation or the documentation needs to be adapted.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/style
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link
Updated by Julian Mair almost 4 years ago
Sure, here is an example of what is happening currently:
Fluid Template:
<f:asset.css identifier="header" href="EXT:package/src/to/header.css" priority="1"/>
<f:asset.css identifier="footer" href="EXT:package/src/to/header.css" priority="0"/>
<f:asset.css identifier="main" href="EXT:package/src/to/header.css" priority="1"/>
<f:asset.css identifier="inline" priority="0">
div { display: none; }
</f:asset.css>
<f:asset.css identifier="inline2" priority="1">
div { display: block; }
</f:asset.css>
<p>... other content</p>
Browser Output (Simplified):
<html>
<head>
<link href="/src/to/header.css" rel="stylesheet" type="text/css">
<link href="/src/to/main.css" rel="stylesheet" type="text/css">
<link href="/src/to/footer.css" rel="stylesheet" type="text/css">
<!-- all link tags are in the header and grouped by 'priority' -->
<!-- 'priority=1' is above and 'priority=0' beneath -->
<style> div { display: block; } </style>
</head>
<body>
<p>... other content</p>
<style> div { display: none; } </style>
</body>
<html>
... and how it should be (according to the docs):
Browser Output (Simplified):
<html>
<head>
<link href="/src/to/header.css" rel="stylesheet" type="text/css">
<link href="/src/to/main.css" rel="stylesheet" type="text/css">
<!-- only 'priority=1' marked tags are in head, the others are before the closing body tag -->
<style> div { display: block; } </style>
</head>
<body>
<p>... other content</p>
<link href="/src/to/footer.css" rel="stylesheet" type="text/css">
<style> div { display: none; } </style>
</body>
<html>
This happens only because of `EXT:frontend/Resources/Private/Templates/MainPage.html`
In that file, the order of the markers '###CSS_LIBS###' and '###CSS_INCLUDE###' are not 'correctly'.
'###CSS_INCLUDE###' has to be right before '###JS_LIBS_FOOTER###' in order to work, as written above.
The original file:
###XMLPROLOG_DOCTYPE###
###HTMLTAG###
###HEADTAG###
###METACHARSET###
###INLINECOMMENT###
###BASEURL###
###SHORTCUT###
###TITLE###
###META###
###CSS_LIBS###
###CSS_INCLUDE###
###CSS_INLINE###
###JS_LIBS###
###JS_INCLUDE###
###JS_INLINE###
###HEADERDATA###
</head>
###BODY###
###JS_LIBS_FOOTER###
###JS_INCLUDE_FOOTER###
###JS_INLINE_FOOTER###
###FOOTERDATA###
</body>
</html>
I assume this is breaking if we change it, isn't it?
Better to just fix it in the docs and let everyone self decide to override the file "MainPage.html" via the TypoScript config `pageRendererTemplateFile`?
The inline styles are handled separately, because they are appended to `jsFooterFiles`
// httpdocs/typo3/sysext/core/Classes/Page/PageRenderer.php:1835
// ...
// append inline CSS to footer (as there is no cssFooterInline)
$jsFooterFiles .= $assetRenderer->renderInlineStyleSheets();
// ...
Updated by Jonas Eberle almost 4 years ago
- Assignee set to Jonas Eberle
- Tags changed from fluid, assets, viewhelper, doc to fluid, assets, viewhelper, doc, assetcollector
Updated by Jonas Eberle over 3 years ago
There is some inconsistency. But I think it has to do with best practices.
We actually could put `<link rel="stylesheet">` to the bottom (above ###JS_LIBS_FOOTER###) yet it is not recommended to have it in `<body>` ("Other usage notes" at https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link). I guess that having stylesheets that late and nested into `<body>` would worsen rendering performance and the Largest Contentful Paint metric.
The AssetCollector was recently added to the official docs https://docs.typo3.org/m/typo3/reference-coreapi/10.4/en-us/ApiOverview/Assets/Index.html
So @Julian Mayr, would you agree that we would need to change these docs (look for `<head>`)?
https://docs.typo3.org/other/typo3/view-helper-reference/master/en-us/typo3/fluid/latest/Asset/Css.html#priority
https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Assets/Index.html
https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/10.3/Feature-90522-IntroduceAssetCollector.html
Updated by Julian Mair over 3 years ago
In the last couple of months, I tried to find a good working solution by myself, but at the end I came to the same conclusion as you: To put JS or CSS files in the <body> tag worsen overall rendering and loading performance. The <head> is nearly always the best place. It's just a matter of priority:
For JS we have `async` and `defer` - so we are good.
For CSS we have no real attribute, just `preload` for modern browsers or some "hacks", like:
- https://web.dev/defer-non-critical-css/
- https://www.filamentgroup.com/lab/load-css-simpler/#the-code
Not perfectly clean, but it works - it just needs the attribute `onload` to be available on the css asset ViewHelper, which it's not the case ATM.
So what we can do is:
1) Yes - to your original question -, we have to make the changes to the docs to get the things straight.
2) More optional (but not breaking IMHO): to add the attribute `onload` to both of the asset ViewHelpers, so Typo3 doesn't block the hacks/solutions, in the first place.
What do you think?
Updated by Jonas Eberle over 3 years ago
Thank you for these links. Honestly I haven't come across defering CSS yet and am happy to learn. But oh my is it an ugly workaround :) If it was prettier and feeling more stable I would have opted for a "defer" option on our API that outputs such HTML.
But such thing could be fully (including the `<noscript>`) implemented in an extension with the Before*RenderingEvents (https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Hooks/Events/Core/Page/BeforeStylesheetsRenderingEvent.html#beforestylesheetsrenderingevent).
You can add the "onload" to the ViewHelpers with "additionalAttributes":
page = PAGE
page.10 = FLUIDTEMPLATE
page.10 {
template = TEXT
template.value (
<f:asset.css identifier="deferCss" href="EXT:styleguide/Resources/Public/Css/styles.css"
additionalAttributes="{onload: 'this.onload=null;this.rel=\'stylesheet\'', rel: 'preload', as: 'style'}"
/>
)
}
With PHP:
\TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(\TYPO3\CMS\Core\Page\AssetCollector::class)
->addStyleSheet(
'deferCssFromPhp',
'EXT:styleguide/Resources/Public/Css/styles.css',
[
'onload' => 'this.onload=null;this.rel=\'stylesheet\'',
'rel' => 'preload',
'as' => 'style',
]
);
This outputs
<link onload="this.onload=null;this.rel='stylesheet'" rel="preload" as="style" href="/typo3conf/ext/styleguide/Resources/Public/Css/styles.css?1597594996" type="text/css" >
That's pretty close to https://web.dev/defer-non-critical-css/ (except the `<noscript>`).
Would that be a good example for the docs?
Updated by Julian Mair over 3 years ago
Holy moly! I've never thought about 'additionalAttributes' and the 'Before*RenderingEvents' before... . Thank you very much for this! ^^
Well ... the 2nd task from my comment above is now not necessary anymore. Eventually it's just about the docs, so others can find such beautiful things.
I would propose:
First, to correct the priority description for the css ViewHelper in the files/links you mentioned.
And secondly, to write somewhere (= that's the hard question now) this two examples for css deferment. Maybe also the link to https://web.dev/defer-non-critical-css/ for further information about the benefit of this use case.
Updated by Jonas Eberle over 3 years ago
I agree.
So, to form a plan:
TODO¶
Correct description of `priority` toggle:¶
Add "defer CSS" example to:¶
- (Fluid) https://docs.typo3.org/other/typo3/view-helper-reference/master/en-us/typo3/fluid/latest/Asset/Css.html
- we should not add it here since it is rendered from the Viewhelper phpdoc. I added a cross-reference.
- (PHP Event) https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Hooks/Events/Core/Page/BeforeStylesheetsRenderingEvent.html
That would also help us to have a "Stylesheets" example since currently we reuse the "JavaScript" example there.
The event could for example rewrite all priority=0 CSS to the https://web.dev/defer-non-critical-css/ - "hack" ;)
Updated by Julian Mair over 3 years ago
Superb!
I would help you, but my time schedule says otherwise. I just can provide you with some phrases, which you (or whoever) can use ... at least something :)
Priority descriptions for viewhelper:¶
In contrary to the JS priority, the CSS assets come always into the <head> tag, but grouped in two separated blocks. The first is 'priority=true', the latter 'priority=false'.
AssetsCollector descriptions (look for <head>) AND Changelog:¶
This paragraph:
It supports best practices for optimizing page performance by having a “priority” flag to either move the asset to the <head> section (useful for CSS in above-the-fold concepts) or the bottom of the <body> tag.
... transformed into this:
It supports best practices for optimizing page performance by having a “priority” flag to either move the asset to the <head> section or the bottom of the <body> tag. But this counts only for the JavaScript priority, the CSS assets come always into the <head> tag, grouped in two separated blocks. The first is 'priority=true', the latter 'priority=false'.
I hope this helps.
Updated by Gerrit Code Review over 3 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67512
Updated by Gerrit Code Review over 3 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67512
Updated by Gerrit Code Review over 3 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67512
Updated by Gerrit Code Review about 3 years ago
Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67602
Updated by Anonymous about 3 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 4b50bec1914062ae6d9311d8207d8074217551e8.
Updated by Jonas Eberle about 3 years ago
- Status changed from Resolved to In Progress
Gerrit closed here automatically. Reopening because we are not completely done with the task yet.
Updated by Anonymous about 3 years ago
- Status changed from In Progress to Resolved
Applied in changeset ac54283f12ab6f6c36024b603920c67175b93831.
Updated by Jonas Eberle about 3 years ago
- Status changed from Resolved to In Progress
Updated by Jonas Eberle over 2 years ago
- Status changed from In Progress to Closed
I am closing this since it is stalled for a long time.
We added the critical parts of information where they were missing.
We can add a better example to https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Hooks/Events/Core/Page/BeforeStylesheetsRenderingEvent.html as an improvement when we feel like it but I think that doesn't reward an open forge issue any more.