🇺🇸United States @luke.leber

Moving back to needs work due to...

1. The issue summary describing the problem but not clearing laying out a proposed resolution.
2. This issue has a visible patch (that doesn't apply to HEAD) -- patch workflow is antiquated and will not run automated tests anymore.
3. This issue having 2 forks (which are hidden by default) -- both of which go different directions than the patch.

All in all, I don't think this can be "needs review". There needs to be a written-down plan and one up to date merge request to be eligible for needing review IMHO.

That all said something definitely needs to be done here. The tap area for the current out of the box experience is too small to pass accessibility audits, so tagging with accessibility.

Back to NR for !11689 (10.5.x) - I'm out of time to open other PR's, we've decided to patch this in as needed.

luke.leber → changed the visibility of the branch 3471642-upgrade-asm89stack-cors-to to hidden.

Add backport plea.

I couldn't actually find any instructions on how to upgrade composer dependencies in core issues, so I just took a wild guess. I'm guessing the automation will bark if it's way off-base.

Cleaned up the IS.

Update title + IS.

Thanks Adam.

Great news -- I'm going to close this one out as outdated. While this still results in a somewhat broken design system on my end, this is now able to be very easily resolved by adding a defensive display property that won't be clobbered by the Experience Builder scaffolding styles!

Thanks to everyone involved upstream.

Woo! Odds are the upstream issue is now fixed. Self-assigning for a re-test (tonight or tomorrow on the plane).

I still have a "broken" test case readily at hand.

Clean up I.S.

Added some example model configuration files to aid in review.

I'll mark as NR for the concept check anyhow.

I've taken a slightly different stab at this in the issue fork. Rather than providing various flags for the \sort function, the action operates at a slightly higher level by allowing users to pick different sorting functions.

Jurgen also mentioned in a slack thread...

I'd like to add that there should also be sorting based on either keys, values, or callbacks. The latter would allow sorting based on properties or other logic when values are complex data types.

This might be a crazy idea, but I think that this could be feasible by linking to another ECA action that acts to compare two values with arbitrary steps!

I know that the actual sorting implementation isn't correct -- but it at least allows the kernel tests to pass.

I'll open a draft PR to get some feedback on this feature request.

Thanks!

luke.leber → made their first commit to this issue’s fork.

loadCSS is more or less abandonware nowadays. Regarding CSP's there are always some complications there, but nothing insurmountable. CSP has a great API that can probably handle anything we throw at it.

https://git.drupalcode.org/project/inline_all_css/-/blob/1.0.x/src/Event... is how I added CSP support to inline_all_css. Granted core wouldn't implement a subscriber, obviously. I guess CSP would have to come up with some solution to deploy in tandem with this feature.

I can reproduce this! I've opened an upstream issue with diff 💬 A crossroads: how to resolve warnings when a module adds new diff plugins? Active to see what the best way to resolve this is.

Thanks for reporting!

luke.leber → created an issue.

I think the proposal in #19 is a rational start, but am slightly concerned that not all business rules can be represented in YML fashion.

I'm still of the mind that power users will require some degree of programmatic interface to check all the boxes that are presently fulfilled by things like LB Restrictions.

Whether these traits exist on the SDC or in a parallel XB-only configuration is the question.

Additionally I'm thinking that adding all this at the slot level in SDC will make it very hard share SDCs across themes/modules.

This is a great point. I do rather struggle to see how SDC's would be able to be mixed and matched across different themes in ways that wouldn't end up "looking weird" from a branding perspective though. My mind immediately went to an example like "I'd like a header from Princeton's design system, a footer from Harvard's, and accordions from Yale's -- which would result in a very odd looking thing from a branding perspective.

Are there any concrete use cases for wanting to mix and match SDC's across different themes? Typically I'd think that there wouldn't be the desire for design systems to be arbitrarily mixed and matched in that fashion.

Ugh related issue != Child issue

Dropping this in from slack to hopefully get a follow-up at some point (in the project management court now):

With ~100ish components on a page, here are some browser performance metrics from a high end laptop:

~45 second time from page load to rendering the XB UI with dev tooling enabled; ~20 seconds w/o.

INP with dev tools ~3-4 seconds. 1.2 seconds w/o

--

I know that XB is still very early in development, but wanted to raise some awareness about potential scalability problems that will naturally be MUCH harder to tackle later as an after-thought -- even moreso once disruptive changes are harder to justify!

griffynh → credited luke.leber → .

Oo, shiny.

Yeah, this particular issue seems to go away with that MR applied.

I don't see much utility in adding render-blocking CSS at the end of <body>. The HTML spec isn't particularly helpful there and it's really a wild west in terms of how user agents should behave. I did some brief chrome testing and it doesn't look like it matters if render-blocking styles are in the head or in the body. First paint seems to be blocked in either case equivalently. Adding the feature to make certain styles non-blocking seems to have a lot stronger of a behavior guarantee than relying on progressive painting.

Something about

    <!-- ... -->
    <link href="style.css" rel="preload" as="style" onload="this.rel='stylesheet'">
    <noscript><link rel="stylesheet" href="style.css"></noscript>
  </body>

has an aura of elegance around it.

luke.leber → created an issue.

I'm on the fence of deferring CSS. It's often the delicate act of striking a balance between LCP and CLS. The list of things that you truly get for free when deferring CSS is pretty small. Of course, dynamic content added via direct user action (ajax stuff?) and non-visual content that can only be made visible by user interaction (the skip to content link visual styles -- not the css that hides it visually by default?) are the only ones that come to mind.

Users can even land on the page footer with the right #fragment in some cases. We provide dozens of jump links to help people link directly to the correct information, which in some cases means that the footer is effectively above the fold.

I tend to lean toward prioritizing CLS optimization where there's a choice (no one likes to see incomplete stuff or content that bounces around).

Also it feels like it's only worth inlining CSS if we also defer all other CSS

100%

I am mildly concerned about how the Drupal off-canvas CSS inline style rewrites would work with this. They were somewhat quirky sometimes with inline_all_css 😥.

Overall, I think that some real world data should be collected with this before arriving on a consensus. While I have a very strong suspicion that it'd lower LCP for most sites/themes, that intuition should be both quantified and peer reviewed.

In the case of the Penn State World Campus theme, inlining all render blocking resources significantly lowered LCP in lab testing (by a complete http round trip 🥳), but we haven't went forward with this in production yet. Sometimes RUM doesn't necessarily agree with what's measured in the lab.

I would second the notion of not enforcing this at the SDC layer due to inherent limitations. I can think up a couple of scenarios where having access to the entire working render tree may be beneficial to effectively enforcing design expectations.

• As a designer, when I design a component that should only be displayed within full-width contexts, a content editor shouldn't be able to add it in a way such that it would display otherwise.
• As an accessibility auditor, a component known to have dark text shouldn't be able to be placed if the closest parent component that ships with a background color is known to have a dark background

Layout builder restrictions offers an API that does offer quite a bit of additional context in ways that make non-trivial business logic able to be enforced. The "in the weeds" bits here are quite niche in nature, and will likely vary from design system to design system. Is providing an API for restriction management / XB tree validation on the table?

This seems like something we can write a test case for. I think before attempting a fix here, we'll need a failing test.

Equivalency to spaceless is rather heavy handed to achieve. Basically there are two preconditions:

1. There can't be any markup passed in via variables (I think we meet this criteria here!)
2. Every {%, %}, {{, }}, etc... have to be converted to use whitespace control operators (i.e. {{-, -}}

I think we'll still need the second point to apply here, but I'm still rather critical of the upstream decision to remove spaceless without a 100% solid replacement (see #1) ☹️.

luke.leber → created an issue.

kristen pol → credited luke.leber → .

Atlanta would be a great space to discuss this. Between LB styles, LB component attributes, and who knows what else in contrib space, how would contrib even coordinate all of these upgrade paths? Would every module do it independently? Only for their third party settings? What about custom stuff too?

I could see some sites needing like 4 or 5 iterations of saving every revision to make this work in contrib/custom...it just seems like a very messy thing for core to put the burden on site owners with.

If it's not an easy upgrade (read: something that non-technical site owners would have to spend $$$ on an agency to custom code a migration for), odds are trust will be damaged. In today's super competitive market, damaging trust shouldn't be downplayed -- especially if migrating to a competitor's solution ends up being cheaper to the business.

$.02 😥

Rather doubtful anything will happen with this as it's a Drupal 7 issue. Seems to have been committed to 8+ already.

I would like to be linked to one other core commit that would force site owners to run multiple content updates on every single revision of every single content entity on their site.

In the past 10 years I cannot think of one. IMO this sets a very dangerous precedent..

What happens if MYSQL goes away mid-update? Race conditions? Many known race conditions exist in core.

kristen pol → credited luke.leber → .

luke.leber → created an issue.

luke.leber → created an issue.

This sort of thing provides unique challenges (note how it's possible to produce contrast-inaccessible variants).

Originally, we sort of "flattened" the options, but found it wasn't flexible enough for our designer. More recently we've taken the approach in our in-house design system to make colors automatically contrast themselves based on their parent contexts. In general, components have light and dark variations set up and the CSS makes sure things stay accessible.

If curious, see https://psu-ooe.github.io/?p=atoms-cta-auto-contrast-deeply-nested (examples) and https://github.com/PSU-OOE/components/blob/main/packages/button-base/src... (driving CSS).

I also did a write-up at https://www.lukeleber.com/blog/2024-07-25-context-aware-components

It's a lot more complex on the CSS end, but even if JavaScript starts hacking and slashing the DOM after the page is built we usually never have to worry about color contrasting.

Food for thought 🙂.

I wasn't aware that more than one condition of a specific type per block was possible to configure. The core block GUI does not allow for that.

Perhaps with custom code or lesser popularity contrib though? That should be easy to resolve, but I'm not sure it's worth doing.

Those are really good points!

I think perhaps our use case (in using a condition plugin to control the presence of global blocks) may be best achieved with a different mechanism -- probably on the block types themselves.

I don't see an elegant way to hit all of the needs of this module as prescribed in the I.S. I'm leaning towards this being a wontfix.

Thanks for the swift response.

Update I.S. - Moving on to some manual testing on the issue fork on a real site :-).

Self-assigning to work on a pull request.

luke.leber → created an issue.

luke.leber → created an issue.

I've opened an upstream pull request that may resolve this linked to the issue that was reported upstream.

Hopefully the fix ends up being as simple as it seems!

luke.leber → created an issue.

Unassigned + sent to NR status

luke.leber → created an issue.

Everyone: Thank you for manually testing the Drupal 11 upgrade.

I'd like to leave an additional comment regarding the explicit dropping of support for Drupal 9.

Given that no testing has been performed against Drupal 9, it's my opinion that it's unlikely that any future enhancements to this module will be tested against Drupal 9 or prior either. Given this, it's even more unlikely that support ranges excluding Drupal 9 would be considered in future merge requests, meaning that unfortunate site owners that are still trapped in Drupal 9 could otherwise be unwittingly walked into WSOD situations. The most responsible thing to do as a community is to drop support so that users stuck on out of support versions of Drupal will not be able to brick their site by an upgrade of this module that couldn't be effectively tested against end of life software.

I'll cut a new release for Drupal 11 support shortly. Thanks again.

We also need to test on the previous major given Drupal 10 is still supported, so the gitlab CI template needs to be updated.

The MR pipelines are presently failing.

A composer.json file is required for phpunit testing in Gitlab CI.

There needs to be a test for this to happen. Happy to accept any test coverage 😇.

I don't have any cycles to do much more than keep the lights on, as I no longer manage any live sites that use this module.

I'll test drive this over the weekend on 11.x. Given there were no manual tests against Drupal 9 and it's end of life, I think it's best to explicitly drop support, leaving ^10.3 || ^11 as the support range.

Claiming that Drupal 9 is still actively supported would be somewhat false advertising 😅.

luke.leber → changed the visibility of the branch 3506838-consider-adding-scheduling to active.

luke.leber → changed the visibility of the branch 3506838-consider-adding-scheduling to hidden.

I plan on opening a pull request this weekend at some point, so self-assigning.

luke.leber → created an issue.

The root cause here is that version 2.0.6 introduced a MAJOR backwards compatibility problem.

Prior to 2.0.6, library dependencies could be relied upon in order to prevent really gnarly race conditions from happening between the GTM JS and Drupal libraries.

This happened because https://www.drupal.org/project/google_tag/issues/3452712 🐛 Possibly script loading/placement issue Fixed significantly changed the loading order in a way that bypasses dependency resolution. As a result, in order to prevent race conditions, any drupal library that is depended on by GTM must also be loaded in the head, and the GTM library definition must be hooked into in order to declare the dependency..

This feature would allow the Inline All CSS → module to continue to live in Drupal 11+.

Feel free to review, but this approach seems very reasonable to me in terms of performance 💪. This operates at a very low level, so it's very likely that this first stab is not 100% perfect.

Business logic encapsulated in a helper service

<?php

namespace Drupal\psu_layout_builder;

use Drupal\Core\Database\Connection;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\layout_builder\Section;
use Drupal\layout_builder\SectionComponent;

/**
 * Helper to facilitate 'additional' to 'third_party_settings' migration.
 */
class SectionComponentTPS {

  /**
   * Service constructor.
   *
   * @param \Drupal\Core\Database\Connection $database
   *   The database connection service.
   */
  public function __construct(
    protected Connection $database
  ) { }

  /**
   * Migrates 'additional' to 'third_party_settings' for the provided table.
   *
   * @param string $table
   *   The database table to migrate.
   * @param array $sandbox
   *   The post-update sandbox.
   *
   * @throws \Exception
   *   😱 God help you if this happens.
   */
  public function migrate(string $table, array &$sandbox, $sections_per_batch = 10000) {
    if (!isset($sandbox['total'])) {
      $query = $this->database->select($table);
      $sandbox['total'] = $query->countQuery()->execute()->fetchObject()->expression;
      $sandbox['current'] = 0;
      if (empty($sandbox['total'])) {
        $sandbox['#finished'] = 1;
        return NULL;
      }
    }

    $query = $this->database
      ->select($table, 'layout')
      ->fields('layout', ['bundle', 'entity_id', 'revision_id', 'langcode', 'delta', 'layout_builder__layout_section'])
      ->range($sandbox['current'], $sections_per_batch);

    $rows = $query->execute()->fetchAll(\PDO::FETCH_ASSOC);
    if (empty($rows)) {
      $sandbox['#finished'] = 1;
      return;
    }
    else {

      // Combine all update queries into a single upsert for performance.
      $upsert = $this->database->upsert($table)
        ->fields(['bundle', 'entity_id', 'revision_id', 'langcode', 'delta', 'layout_builder__layout_section'])
        ->key('revision_id');

      foreach ($rows as $row) {
        /** @var \Drupal\layout_builder\Section $section */
        $section = unserialize($row['layout_builder__layout_section'], ['allowed_classes' => [Section::class, SectionComponent::class]]);
        foreach ($section->getComponents() as $component) {
          // Set might be deprecated, but it's hella efficient.
          // Don't run this code on Drupal 12, eh?
          $component->set('thirdPartySettings', $component->get('additional') ?: []);
          $component->set('additional', []);
        }
        $row['layout_builder__layout_section'] = serialize($section);
        $sandbox['current']++;
        $upsert->values($row);
      }

      $upsert->execute();
    }

    if ($sandbox['current'] >= $sandbox['total']) {
      $sandbox['#finished'] = 1;
    }
    else {
      $sandbox['#finished'] = ($sandbox['current'] / $sandbox['total']);
    }
    return new TranslatableMarkup('@count of @total' . ' layout sections processed.', [
      '@count' => $sandbox['current'],
      '@total' => $sandbox['total'],
    ]);
  }

}

Service configuration boilerplate

services:
  Drupal\psu_layout_builder\SectionComponentTPS:
    class: Drupal\psu_layout_builder\SectionComponentTPS
    autowire: true

Sample post update hooks for *one* entity type

/**
 * Migrate 'additional' to 'third_party_settings' for nodes.
 */
function psu_layout_builder_post_update_tps_nodes(array &$sandbox) {
  return \Drupal::service(SectionComponentTPS::class)->migrate('node__layout_builder__layout', $sandbox, 10000);
}

/**
 * Migrate 'additional' to 'third_party_settings' for node revisions.
 */
function psu_layout_builder_post_update_tps_node_revisions(array &$sandbox) {
  return \Drupal::service(SectionComponentTPS::class)->migrate('node_revision__layout_builder__layout', $sandbox, 10000);
}

Let's do this thing @ 10,000 records per batch.

www-data@9218f372c877:~/html$ time drush updb
 -------------------- -------------------- ------------- ---------------------- 
  Module               Update ID            Type          Description           
 -------------------- -------------------- ------------- ---------------------- 
  psu_layout_builder   tps_node_revisions   post-update   Migrate 'additional'  
                                                          to                    
                                                          'third_party_setting  
                                                          s' for node           
                                                          revisions.            
  psu_layout_builder   tps_nodes            post-update   Migrate 'additional'  
                                                          to                    
                                                          'third_party_setting  
                                                          s' for nodes.         
 -------------------- -------------------- ------------- ---------------------- 


 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > 

>  [notice] Update started: psu_layout_builder_post_update_tps_node_revisions
>  [notice] 10000 of 208192 layout sections processed.
>  [notice] 20000 of 208192 layout sections processed.
>  [notice] 30000 of 208192 layout sections processed.
>  [notice] 40000 of 208192 layout sections processed.
>  [notice] 50000 of 208192 layout sections processed.
>  [notice] 60000 of 208192 layout sections processed.
>  [notice] 70000 of 208192 layout sections processed.
>  [notice] 80000 of 208192 layout sections processed.
>  [notice] 90000 of 208192 layout sections processed.
>  [notice] 100000 of 208192 layout sections processed.
>  [notice] 110000 of 208192 layout sections processed.
>  [notice] 120000 of 208192 layout sections processed.
>  [notice] 130000 of 208192 layout sections processed.
>  [notice] 140000 of 208192 layout sections processed.
>  [notice] 150000 of 208192 layout sections processed.
>  [notice] 160000 of 208192 layout sections processed.
>  [notice] 170000 of 208192 layout sections processed.
>  [notice] 180000 of 208192 layout sections processed.
>  [notice] 190000 of 208192 layout sections processed.
>  [notice] 200000 of 208192 layout sections processed.
>  [notice] 208192 of 208192 layout sections processed.
>  [notice] Update completed: psu_layout_builder_post_update_tps_node_revisions
>  [notice] Update started: psu_layout_builder_post_update_tps_nodes
>  [notice] 6082 of 6082 layout sections processed.
>  [notice] Update completed: psu_layout_builder_post_update_tps_nodes
 [success] Finished performing updates.

real    0m15.490s
user    0m7.965s
sys     0m0.749s

15 seconds to churn through over 210,000 records seems agreeable to me!

I wonder if the gist in #239 could be optimized by operating on the layout builder section storage tables and bypassing the Entity API? The SectionStorage can be loaded and saved independently of the content entity it's attached to I believe.

Generally, saving content entities is MAJORLY slow comparatively. I plan on running additional benchmarking on this at some point as we have over a quarter million sections to churn through when this finally lands. 🤞

Unassigned -- I have no idea where to even start to look now!

This is a pretty gnarly issue. From what I can gather, these are the enabling criteria for the mix-up in CSS order...

1. The SDC must be provided by a theme
2. The SDC must define a libraryOverrides that adds a dependency.

The result is that the dependency is loaded after the Experience Builder preview library 🤯. I'm not sure what the best way to share an example of this is.

Any suggestions?

This change doesn't seem to add much value. It simply restates the module's description.

I'll set it back to needs work for now, but unless there's anything helpful information for end-users to add via this hook, it won't be merged.

My recommendation is to gather feedback from users and see what they're having trouble with, if anything, before making an attempt at a hook_help implementation.

General +1 from me on this. In my evaluation of the CONTRIBUTING file, I didn't consider that setting up a working SSH key with Gitlab was a prerequisite. 😮

Might be even better to state the root cause and add a link to the doc page for setting up an SSH key?

You probably do not have a valid SSH key configured for accessing repositories on drupal.org. See <page> for setting one up or explore the following alternative options.

Until the README stops pointing to the CONTRIBUTING file, this seems safest. Ultimately there will probably have to be different instructions for people wanting to contribute/commit to the project and for those that are simply trying to casually use a pre-release to see what's coming.

Shouldn't a metatag integration of sorts be used for this? 😰

luke.leber → created an issue.

Update I.S. with more info.

luke.leber → created an issue.

luke.leber → created an issue.

smustgrave → credited luke.leber → .

Setting back to Needs Work. Here's a slack chat to document where things are.

balintbrews
23 minutes ago
@lleber
I might be doing something wrong, but I can’t reproduce the issue on 0.x anymore.

lleber
:male-scientist: 23 minutes ago
I'll double check, but I was just able to around 8:30 :sweat_smile:

lleber
:male-scientist: 16 minutes ago
Ah, so it may be a dependency based thing :open_mouth:.
I added an example-flexbox-break component and here's the CSS inheritance...

(XB styles override example-flexbox-break component styles here)

9:38
Then I added a header (custom) component and here's the inheritance!

(header component styles override XB styles here)

9:39
The [data-xb-slot-id] selector seems to have nondeterministic loading order, meaning the example in the I.S. seems to work, but another component does not!
9:40
I'll dig a little deeper to see if I can sleuth a better example.

balintbrews
13 minutes ago
I can see that you have CSS aggregation turned on. Maybe that comes into play as well?

lleber
:male-scientist: 13 minutes ago
I can try disabling it, but the only reason the example component in the I.S. seems to work is because its display: flex is being overridden.

balintbrews
13 minutes ago
Yes, I saw that happening on my end as well.

lleber
:male-scientist: 12 minutes ago
I do think that if the SDC had a dependency injected on the preview CSS, it'd straighten itself out, but that seems odd.

balintbrews
12 minutes ago
Yeah, we don’t want that.
9:44
@lleber
I’m leaning towards just committing the width: 100% you suggested, I can’t see how that would hurt us. And it may help you now. :slightly_smiling_face: If we see more of these styling issues, we can do a deeper investigation. What do you think?

lleber
:male-scientist: 8 minutes ago
I think it'd definitely help in the short-term, but it may be a harbinger of deeper issues. I think it has to do with the SMACSS layer the libraries are loaded at.
9:46
The component displaying problems is registered at the component layer. Trying to figure out which layer SDCs are registered at.
9:46
I wonder if the XB styles should load at the lowest layer so that everything can override them? Or will that cause issues (see: layout builder off-canvas :smile:)? (edited)
New

balintbrews
2 minutes ago
Our styles in preview.css are unlayered, so they will take precedence over other layers.

lleber
:male-scientist: 1 minute ago
Hm..then I wonder how our custom header is winning there. I'm out of time this morning to debug, but I'm unblocked to find a root cause now.

I think a deeper investigation is needed to see what exactly is going on.

I.S. update + NR

This is certainly still an issue, except now the .xb--sortable-slot-empty-placeholder class doesn't specify a minimum width, and will collapse down to 0px wide if the parent element has display: flex.

I think that the safest bet here is to give the .xb--sortable-slot-empty-placeholder a width of 100%. The rationality here being that there should only ever be one empty placeholder per slot, right? As soon as a component is added, the placeholder goes away. I can't think of any reason why we wouldn't want the placeholder's drop-zone to be smaller than it could be!

luke.leber → created an issue.

https://www.drupal.org/project/experience_builder/issues/3491021 ✨ Leverage HTML comment annotations, remove wrapping div elements Active has landed and this issue needs re-triaged. Self-assigning and will get around to this tomorrow between 8:30 and 5:00 ET.

Whenever I see a call to wait on a lock I get a chill up my spine.

From a cursory glance, this change could result on a ton of processes spinning/waiting to acquire a lock under a heavy load, couldn't it?

I think this one should be subject to some load testing on a larger site.

A first step is probably to get all of the changes needed in place that Gitlab needs to get all of the jobs green. That's a standardized set of tooling that everyone can share. I think https://www.drupal.org/project/juicebox/issues/3351683 📌 [PP-1] Resolve all coding standard violations Postponed is the place for that work I think. Any commit made to the issue fork over there will automatically run those tests.

Koustav Mondal self-assigned that issue to them self, but hasn't made any commits in a week, so IMO it's fair game for anyone to commit to.

Things like phpstorm configuration will probably vary from person to person, but the gitlab tests are a shared configuration between everyone. They should probably be resolved first before making any other changes so that everyone's on the same page with the standard checks.

After the gitlab checks are all passing, so long as further changes don't make the checks fail, they can probably be applied in a follow-up.

Here's something for us to run temporarily in the new year...

diff --git a/core/assets/scaffold/files/default.settings.php b/core/assets/scaffold/files/default.settings.php
index cd364bb00d..751a5de5ff 100644
--- a/core/assets/scaffold/files/default.settings.php
+++ b/core/assets/scaffold/files/default.settings.php
@@ -811,6 +811,14 @@
  */
 $settings['migrate_node_migrate_type_classic'] = FALSE;
 
+/**
+ * Aggregated asset error logging.
+ *
+ * This is used to inform site owners when CSS or JS assets fail to be included
+ * in an aggregated asset.
+ */
+$settings['log_aggregated_asset_errors'] = FALSE;
+
 /**
  * The default settings for migration sources.
  *
diff --git a/core/lib/Drupal/Core/Asset/CssOptimizer.php b/core/lib/Drupal/Core/Asset/CssOptimizer.php
index 8f2d910250..61d00ea129 100644
--- a/core/lib/Drupal/Core/Asset/CssOptimizer.php
+++ b/core/lib/Drupal/Core/Asset/CssOptimizer.php
@@ -5,6 +5,7 @@
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
 use Drupal\Core\File\FileUrlGeneratorInterface;
+use Drupal\Core\Site\Settings;
 
 /**
  * Optimizes a CSS asset.
@@ -145,7 +146,11 @@ public function loadFile($file, $optimize = NULL, $reset_base_path = TRUE) {
     // stylesheets in their .info.yml file that don't exist in the theme's path,
     // but are merely there to disable certain module CSS files.
     $content = '';
-    if ($contents = @file_get_contents($file)) {
+
+    // Temporary hack for debugging impossible to diagnose aggregation issues...
+    $contents = Settings::get('log_aggregated_asset_errors') ? file_get_contents($file) : @file_get_contents($file);
+
+    if ($contents !== FALSE) {
       // If a BOM is found, convert the file to UTF-8, then use substr() to
       // remove the BOM from the result.
       if ($encoding = (Unicode::encodingFromBOM($contents))) {

Unrelated - there should probably be a strict comparison against the file_get_contents result here -- otherwise some false positives can slip through depending on the file contents 😱!

Ideally when we know that a corrupted asset is going to be generated, we'd want to issue an HTTP/500 to prevent the asset from becoming cached by downstream infrastructure, but that definitely can't happen without breaking huge swathes of sites.

Given the uncertainty about what's supported or not in #2, I think maybe we could add this behind an opt-in behavior to allow for a graceful path to deprecation?

I'm sure that a lot of sites out there have missing assets that's considered very, very normal.

I think this one might be dependent on how https://www.drupal.org/project/experience_builder/issues/3491021 ✨ Leverage HTML comment annotations, remove wrapping div elements Active pans out.