Update issue summary with a more generalized proposal.
I'm not sure that a preconnect would help with LCP at all given the module loads the script async. Loading async removes the JS from the critical render path.
The best thing that the Google Tag module can do to help with improving LCP is to inline the first party JS (or add the async or defer attribute) to it.
Preconnecting to the third party that's already loading async won't help anything unless the LCP element is being loaded from tag manager -- which would be an even bigger problem 😅.
Metrics from a homepage run on Olivero. Test repeated 5x with various throttling applied, results averaged.
As you can see, the results of non-throttled "fast and fortunate" traffic yields negligible gains, but as devices and connection speeds decrease, there's a stark improvement.
Here's a similar metric with a single asset remaining in the critical render path (simulated by dropping a hard-coded link to an empty CSS file in <head>...
This leads to the conclusion that it isn't necessarily how much CSS is "critical" or inlined, but rather the mere presence of ANY render-blocking CSS that is the main contributor to LCP timing as device and network quality decreases.
luke.leber → created an issue.
It's super ugly, but the inline_everything branch might be good enough to take rough benchmarks with?
The one thing it has going for it is that the tests that matter most here don't need an optimal back-end setup. The metrics that matter are all in the browser.
I didn't mean to update !10930 :-(...
I'm going to take a stab at an alternative branch here given !10930 is already being used by client sites.
A bit more info on Acquia Cloud Next here.
Individual response headers are indeed limited to 8kb and exceeding this limit results in an immediate white screen of death. ☠️
When splitting things into multiple headers, it appears that Acquia's infrastructure does not fail, but instead collapses the headers into a single one, comma-separating the values.
For example:
Cache-Tags: ...almost 8k of them, abcd
Cache-Tags: dcba, ...the rest
Is transformed into
Cache-Tags: ...almost 8k of them, abce, dcba, ...the rest
I don't know if this behavior is completely kosher with an RFC or not.
Furthermore, the individual header limit on ACN seems to be slightly less than the advertised 8k. Casual testing indicates that things start failing at 8188 bytes. 🤷
Nevermind! It looks like SSR mode is an option!
I'll have to look a bit closer but the star-rating.js library seems to be a bit susceptible to layout shifting.
Ideally, the markup for an element would be server-side rendered so there aren't any DOM reflows on the client side.
UUID+hostname seems agreeable to me. Should be easy enough to improve upon iteratively if needed.
There's no issue with using multiple inputs for a more unique hash if it catches the 80%.
I think that cache collisions here might actually warrant a meta issue because there's just so much complexity involved. If Drupal didn't have the potential for such large headers this would be a lot easier to solve holistically.
Given the response header limitation for Cloudflare may have increased, it's very likely that upstreams will start hitting resource limitations now.
Acquia is now capped at 8kb for their Cloud Next platform.
https://docs.acquia.com/acquia-cloud-platform/known-issues-cloud-next#se...
I think this almost has to be user-configurable (and enforced?) given all hosting is not equal.
luke.leber → created an issue.
I can confirm that some Acquia enterprise environments handle the Client IP restoration as a platform feature.
luke.leber → created an issue. See original summary → .
Manually posting automated tooling output and screenshots of applying patches isn't really a repeatable / trustworthy way to approach this.
Setting back to NW because the drupal.org automated test runner should be the arbiter of success here -- not local development systems.
Remove noisy bot files.
I gave !5 a test drive against vanilla 11.0.x and 11.2.x installs successfully.
Also committed a test-only fix straight to the project update bot only branch.
Given the nature of the change, I think it doesn't preclude me from a self-RTBC.
luke.leber → changed the visibility of the branch 1.0.x to hidden.
luke.leber → changed the visibility of the branch drupal-11-compat-with-tests to hidden.
luke.leber → made their first commit to this issue’s fork.
Seems pretty outdated to me. See d11 issue.
danrod → credited luke.leber → .
Thanks, everything should be set now. The mobile add maintainer experience on drupal.org is a bit odd but I think I got it.
Let me know if you need anything else, thanks!
Hey Daniel, sure thing. If you'd like to step up please open a task issue. Looking at your account history I have no questions off hand.
This module is not in use by my employer currently and I can't spend hours maintaining it anymore.
re: #90 - honestly I think that's alright. Sites are no worse off than they are now and given the age of this issue, IMO it should be landed as-is. It at least prevents the situation from happening to new users even if certain rare(?) tempstores aren't automatically flushed.
Don't let perfect be the enemy of good here. 🙏
I think that the root of this problem is that the original intent of #3377420 was to target the fallback image.
The <img> element is only a fallback when a <picture> tag is wrapped around it. Drupal's responsive image handling wraps a picture tag around markup conditionally. #3377420 also affects images that are not used in a fallback capacity.
Example configuration:
uuid: 70999a45-bb3c-41f7-9886-ba1fe271b26f
langcode: en
status: true
dependencies:
config:
- image.style.blog_featured_media_l
theme:
- wcstudent
id: blog_featured_media
label: 'Blog featured media'
image_style_mappings:
-
image_mapping_type: image_style
image_mapping: blog_featured_media_l
breakpoint_id: wcstudent.default
multiplier: 1x
breakpoint_group: wcstudent
fallback_image_style: '_empty image_'
In this case, the responsive image style only has a single child image style -- which causes Drupal Core to omit the wrapping <picture> tag.
Example markup before #3377420
<img srcset="/sites/default/files/styles/blog_featured_media_l/public/2025-06/need-help-blog.png.webp?itok=HHMwicZj 1x" width="1200" height="900" src="data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" alt="a colorful note that reads "need help?" " loading="lazy">
After
<img srcset="/sites/default/files/styles/blog_featured_media_l/public/2025-06/need-help-blog.png.webp?itok=ikfUUuon 1x" width="1" height="1" src="data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" alt="a colorful note that reads "need help?" " loading="lazy">
The result is a pretty significant B/C break that makes the image inaccessible to sighted users.
I think you may be correct in that cache busting will happen automatically if a version is omitted.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
Learn something new every day!
Add related issue that caused this.
luke.leber → created an issue.
Setting an official precedent for JIT updates is a great idea, especially given the pending majorly disruptive issues for Layout Builder.
I've done more than a bit of thinking on this and I think that the existing sequential update framework for database updates in general makes for a good basis. If one thinks about it, JIT upgrades are more or less an unrolled version of that, right?
It makes a lot of sense to provide a JIT equivalent that can handle untold millions of records without needing to worry about performance.
Module uninstall requirements can likely handle the more...otherwise prohibitively expensive computations of when it's "safe" for a site to remove a JIT step, yes?
Nowadays ECA is also an option to accomplish this. 🙂
I think someone from the security team should also chime in here given that inlining <style> tags here is inherently more dangerous than loading an external source.
Injecting a </style> somewhere in a CSS asset could lead to a new kind of XSS in core (or otherwise unintentional content injection).
Along side #21, it seems to me that the most easy to use and least prone to error mechanism could be to add a way to flag CSS that is guaranteed to never be critical and load that outside of the critical path.
For example, Drupal off canvas, parts of menus that are only visible on user activation, contextual, skip links, and if custom theme owners are confident enough, anything else they want to flag.
That seems very doable in a follow-up. It might be a good next step to tally up what proportion of core styles are not critical to get a sense of what the long term ROI would be from pursuing #21. That might be a bit time consuming though.
I am mildly confused why this can't be preprocessed back in normally?
/**
* Implements hook_preprocess_HOOK() for form_element__datelist.
*/
function my_theme_preprocess_form_element__datelist(array &$variables) {
foreach (Element::children($variables['element']) as $child) {
$variables['element'][$child]['#title_display'] = 'invisible';
}
}
should work
Hey folks!
I've re-targeted this issue against 2.x-dev. Good news...there's now automated test support.
If someone were to provide a failing test that proves this is an issue it can likely proceed further than it has the past 4 years :-).
Thanks Kyle.
Merged into 2.x and cherry-picked back to 2.0.x. I'll be cutting 2.0.0 stable shortly and will make 2.0.x the recommended version, but will leave 8.x-1.x supported until the end of Drupal 10 support.
Highly doubtful that this module will see another commit until Drupal 12 though.
luke.leber → changed the visibility of the branch project-update-bot-only to hidden.
This is NR (for real) now for !4. Green pipelines and adjusted target branch.
@kyle | @brianne | @zach - any chance of a review for this? Thanks.
luke.leber → made their first commit to this issue’s fork.
Thanks, I plan to open a pull request with an automated test and the appropriate gitlab CI configuration tomorrow and get someone from our internal team to review it.
In all honesty, I haven't had any sponsored time to spend on XB since Atlanta.
I would be curious to hear what Pierre thinks since his SDC advice of iterating slots (versus pursuing dynamic slots is what led me to discover the hacky workaround.
It does seem a very, very fragile state of affairs that iterating a slot's components and injecting some markup can cause XB's front-end to fail by using only well supported twig. I'm assuming that a simple preprocess could do the same.
I do think that this quirk will be fairly commonplace to find in the wild.
smustgrave → credited luke.leber → .
So to summarize the immediate problem to move this forward, when trying to determine if something is an image, there's a string input that's basically in a random format. It might be a scheme://path.extesion?query#fragment with any part present, not present, or even malformed. Anticipate that the string might even be something like a string of random emojis and some unprintable / null control characters, or even characters that aren't allowed in a URL at all that may or may not be encoded - Some of these methods seem to be general utility methods which may be called outside of Mime Mail given that the utility class is not marked @internal!.
This string has to be either:
- Massaged into something that the mime type guesser won't choke on
- Assumed to probably not be an image if things are too messed up to pass to the guesser
Even given Drupal's URL utilities, that's likely extremely tricky as this issue stands. IMO it might be worth checking the various call sites and seeing if there's an opportunity to normalize things. That would of course likely need a B/C break in the utility class, but I do fear that tackling the problem naively may be a very disturbing game of whack-a-mole in the wild. Passing a nullable \Drupal\Core\Url object into the various utility methods would make this a trivial problem to solve.
luke.leber → created an issue.
luke.leber → made their first commit to this issue’s fork.
luke.leber → made their first commit to this issue’s fork.
luke.leber → created an issue.
This change will need a post-update hook to re-save existing configuration.
There's another issue at play here as well. With the syslog module enabled, calls to drupal_flush_all_caches resolves the asset.js.collection_optimizer service, which resolves about 5 other services until it eventually gets to creating a Syslog object. This object causes a cache_config rebuild before drupal_flush_all_caches is ready to start rebuilding.
Specifically, the drupal_static and to a lesser extent the twig caches will be stale / corrupt when this rebuild happens. The state of Drupal is kind of in a weird half-flushed state where all of the bins are empty, but drupal_static isn't.
Crazy idea...but does it really matter if the tempstore entity is updated if the entity was saved with LB? Seems to me at face value that's a premature optimization. Can the route check just be removed...?
2) For now the expectation is that more complex use cases (like tabs or sliders) will have to be coded for by the component developer making use of that window.isInsideExperienceBuilder
I think this could work for the most part if there was sufficient render-time context to conditionally render additional JS. From the limited understanding I have of the semi-decoupled theme engine, that seems possible, so 🥳.
I apologize that I no longer have any officially sponsored time (9-5) to delegate to this effort, but it was a tenuous arrangement from the start. I'll endeavor to respond to issues I'm already involved in until closed though on my own time.
I would like to draw some attention back to the gif at the end of #7. While a naive "in_preview" works for simple accordion-like controls with Boolean states, things like tabs, which could have multiple mutually exclusive states remains a question mark.
To accomplish this in layout builder, our team currently injects additional JS on top of what normally runs on the front-end component to keep track of state.
Admittedly it is a bit clunky as there are massive layout shifts on re-paint though...so I wouldn't read too much into how we "solved" it with layout builder.
Yep! That should do it. I'm not 100% positive on the rationality behind it, but it's a very common pattern for when changes to a module (or core) require a cache flush.
If I had to guess it's so that site owners will see that database updates are required in their dashboards, and when they run said updates, the caches naturally flush out.
Hey,
The call to Html::getUniqueId should prevent duplicate IDs 🤔.
That's mainly why I'm interested in how this is actually happening. I wasn't able to reproduce this. The only thing I keep coming back to is if perhaps your theme or module is also using that ID without wrapping it in Html::getUniqueId?
If the uniquification is broken, we would have to fix it. Removing that ID would be a backwards compatibility breakage that can't happen until a new major is released.
It's my understanding that the mere presence of a post-update hook is sufficient to ensure that caches are flushed appropriately. I don't think we need to call the cache flushing function manually inside.
Almost there for 1.x🙂.
Regarding the 2.x branch and tests, it looks like Drupal 11 has indeed broken some functionality that will need to be addressed 😞.
Best to stick with the stable release until that bag of worms can be resolved.
I gave this a manual test (via the attached model) and everything still seems to be in order. Switching back to NR - thanks Jürgen!
luke.leber → created an issue.
I think the concept is useful, but sometimes the customization can't be hard-coded as data attributes, but rather rely on runtime information.
One example is for the international telephone element. See https://git.drupalcode.org/project/webform_intl_tel_national_mode/-/blob... for a somewhat heavy handed fix for how most UX problems were resolved.
Were there a client side event to tap into, it'd make things a lot easier to tweak. I would LOVE to take a holistic pass on the telephone input given how many quirks there are, and if there was more robust options handling, that'd be even easier.
Moving back to needs work due to...
- The issue summary describing the problem but not clearing laying out a proposed resolution.
- This issue has a visible patch (that doesn't apply to HEAD) -- patch workflow is antiquated and will not run automated tests anymore.
- 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?