Skip to content
Snippets Groups Projects

Resolve #3499829 "Support inlining critical"

Open Resolve #3499829 "Support inlining critical"
issue/drupal-3499829:3499829-support-inlining-critical into 11.x
5 unresolved threads
Open Lee Rowlands requested to merge issue/drupal-3499829:3499829-support-inlining-critical into 11.x
5 unresolved threads

Closes #3499829

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
154 */
155 protected function inlineCssFile(string $path): ?array {
156 $path = \strtok($path, "?");
157 if (!\is_string($path)) {
158 return NULL;
159 }
160 $file_name = \sprintf('%s/%s', DRUPAL_ROOT, $path);
161 if (\file_exists($file_name)) {
162 $contents = \file_get_contents($path);
163 if (!\is_string($contents)) {
164 return NULL;
165 }
166 return [
167 '#type' => 'html_tag',
168 '#tag' => 'style',
169 '#value' => Markup::create($contents),
  • Théodore Biadala
    Théodore Biadala @nod_ started a thread on the diff
    • core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
    147 * Turn a file into a renderable <style> tag.
    148 *
    149 * @param string $path
    150 * File URL to turn into inline CSS.
    151 *
    152 * @return array|null
    153 * Return NULL if the file does not exist.
    154 */
    155 protected function inlineCssFile(string $path): ?array {
    156 $path = \strtok($path, "?");
    157 if (!\is_string($path)) {
    158 return NULL;
    159 }
    160 $file_name = \sprintf('%s/%s', DRUPAL_ROOT, $path);
    161 if (\file_exists($file_name)) {
    162 $contents = \file_get_contents($path);
  • Théodore Biadala
    Théodore Biadala @nod_ started a thread on the diff
    • core/themes/olivero/olivero.libraries.yml
    2 2 version: VERSION
    3 3 css:
    4 4 base:
    5 css/base/fonts.css: {}
    6 css/base/variables.css: {}
    7 css/base/base.css: {}
    5 css/base/fonts.css: { attributes: { critical: true }, preprocess: false }
    • Théodore Biadala
      Théodore Biadala @nod_
      Maintainer

      I'd add a new top level item, the "critical" attribute is not a thing in the HTML spec.

      Suggested change
      5 css/base/fonts.css: { attributes: { critical: true }, preprocess: false }
      5 css/base/fonts.css: { preprocess: false, inline: true }

      not sure about using "inline", might be leaking too much implementation details

    • Andrey Postnikov
      Andrey Postnikov @andypost
      Developer

      why preprocess is required to be provided if inline true?

    • Théodore Biadala
      Théodore Biadala @nod_
      Maintainer

      Fair, I was thinking about not minifying the file but we already have a minified: true if needed

    • Please register or sign in to reply
  • Andrey Postnikov
    Andrey Postnikov @andypost started a thread on the diff
    • core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
    138 // @todo Defer all non critical CSS -
    139 // https://www.drupal.org/project/drupal/issues/2989324
    140 $elements[] = $asset;
    141 }
    142
    143 return \array_filter($elements);
    144 }
    145
    146 /**
    147 * Turn a file into a renderable <style> tag.
    148 *
    149 * @param string $path
    150 * File URL to turn into inline CSS.
    151 *
    152 * @return array|null
    153 * Return NULL if the file does not exist.
  • Luke Leber
    Luke Leber @lleber started a thread on the diff
    • core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
    154 */
    155 protected function inlineCssFile(string $path): ?array {
    156 $path = \strtok($path, "?");
    157 if (!\is_string($path)) {
    158 return NULL;
    159 }
    160 $file_name = \sprintf('%s/%s', DRUPAL_ROOT, $path);
    161 if (\file_exists($file_name)) {
    162 $contents = \file_get_contents($path);
    163 if (!\is_string($contents)) {
    164 return NULL;
    165 }
    166 return [
    167 '#type' => 'html_tag',
    168 '#tag' => 'style',
    169 '#value' => Markup::create($contents),
    • Luke Leber
      Luke Leber @lleber

      This has security implications.

      Things like

      .some-class {
</style><script>alert('oh noes');</script>

      would have previously been harmless but would now result in script execution.

    • Please register or sign in to reply
    Please register or sign in to reply