Account created on 26 December 2007, almost 18 years ago
  • Web Architect, Developer, Themer at Bluespark 
#

Merge Requests

More

Recent comments

🇪🇨Ecuador jwilson3

Issue : In Mobile view Im not able to scroll

Thank you for the review @divya.sejekan!

I decided to revert the initial approach and try something different: position: fixed. for the image on desktop.

This approach does not impact scroll behavior on mobile, plus has the added benefit that the scrollbar appears at the far left edge of the screen, just as before apply the patch does. I wasn't much of a fan of having the scrollbar in the middle of the screen on desktop.

This seems to work much better now.

Feedback and reviews welcome.

Thanks.

🇪🇨Ecuador jwilson3

Thanks for getting this in! 🎉

🇪🇨Ecuador jwilson3

Fixed in release 2.2.7

🇪🇨Ecuador jwilson3

jwilson3 created an issue.

🇪🇨Ecuador jwilson3

I'm with you @ressa. Keeping regex, but making it more approachable makes sense.

One way to make the syntax less daunting, is using # as delimiters (no \/'s then) and an example can be seen in the README MR.

Another option could be to remove the delimiter completely, and handle proper escaping, based on whatever delimiter we decide to use, leaving that part internal to the module.

For example:

/.*\.asp/
/.*\.jsp/
/\/blog_edit\.php/
/\/blogs\.php/
/\/wp-admin.*/
/\/wp-login.*/
/\/my_blogs/
/\/system\/.*\.php/
/.*systopice.*/
/.*login.json/
/\/episerver.*/

Becomes:

.*\.aspx?
.*\.jsp
/blog_edit.php
/blogs.php
/wp-admin.*
/wp-login.*
/my_blogs
/system/.*\.php
.*systopice.*
.*login.json
/episerver.*

Much easier to read.

🇪🇨Ecuador jwilson3

The goal here is to centralize Honeypot administration under a single UI where you'd expect to see it: on the Honeypot admin page.

In my initial investigation, I did not find the global /admin/structure/webform/config (that page is incredibly long!).

Setting aside the time restriction bug you're seeing – which sounds like it should be a separate follow-up – there might be two possible approaches to improve this MR

1. Expose the Webform global settings toggle in two places in the UI:

  • At the top of /admin/config/content/honeypot, where you'd expect to see it.
  • At the bottom of /admin/structure/webform/config under Third Party Settings.

This way we can remove the need for an additional configuration. I understand that such an implementation might be too tightly-coupled, and would complicate the form submit handler, so perhaps the better solution is ...

2. Add help text to the Honeypot config page

To enable Honeypot protection for all Webforms, go to:<br>
<strong>Webforms → Configuration → Third Party Settings</strong> 
(<a href="/admin/structure/webform/config#edit-third-party-settings-honeypot">open settings</a>).

If you prefer, you can also configure Honeypot individually for each Webform under its own Third Party Settings.
🇪🇨Ecuador jwilson3

There are before/after screenshots in the IS, from the raster PNG to the SVG. The approach taken on this issue is exactly the same approach taken on the loading and throbber icon issues, whereby we're not really introducing redesign, we're simply updating the format of the existing files rom raster to vector, so as not to have to spend time bikeshedding redesign, and getting this into core in the next immediate major release.

Splitting these apart into separate issues feels like it would open the door for more cat herding.

I propose keeping this on a single issue, and using the following steps to take before/after screenshots (untested):

  1. Install Drupal with the Standard profile.
  2. Add a multi-cardinality file field to the Article content type that allows upload of each of the file mime types.
  3. Configure field display to display the generic file widget.
  4. Create a node and upload a bunch of files with different file extensions. The files could be zero byte and just be the correct file extension.
  5. Take before/after screenshots of the rendered node.

There would be some amount of iteration, testing, and exploration to ensure we establish a definitive list of file extensions that map to each mime type.

🇪🇨Ecuador jwilson3

Whoops, forgot to update status. This was merged to 2.0.x branch, but not released yet in an official version.

🇪🇨Ecuador jwilson3

Thanks for clarifications. So this is not the same issue as 🐛 Stylesheet missing on /user/register when 403 Needs review , but its in the same vein and the fix there would likely be influenced in some way by a fix for this issue.

🇪🇨Ecuador jwilson3

I just tested on Gin Login 2.1.x branch with a clean standard Drupal 11 install (via ddev-drupal-contrib), but was unable to reproduce the issue. I also sanity checked same behavior on MR 48 for aforementioned issue #3547364, and saw no adverse impact.

Maybe this is something specific to do with Drupal CMS?

I'm attaching some screenshots as evidence that it works as designed with a clean standard Drupal 11 install:

✅ 403 page left empty

✅ 403 page set to /user/login

🇪🇨Ecuador jwilson3

Is this the same (or similar) issue as 🐛 Stylesheet missing on /user/register when 403 Needs review ? The steps to reproduce sound quite different, but maybe the MR I wrote there would work to solve this issue too? Please check! Thanks.

🇪🇨Ecuador jwilson3

MR ready for review.

Before:

After:

🇪🇨Ecuador jwilson3

Long User Register form before MR:

Long User Register form after applying MR:

🇪🇨Ecuador jwilson3

jwilson3 created an issue.

🇪🇨Ecuador jwilson3

jwilson3 created an issue.

🇪🇨Ecuador jwilson3

Instead of uploading these images for every site, it'd be simpler if we could simply point them all to the same path, e.g., /themes/custom/login-logo.png and /themes/custom/login-wallpaper.jpg

My understanding is that the "Logo > Path to custom logo" and "Wallpaper > Path to custom image" fields satisfy this behavior already. What am I missing?

Currently, trying to set the path to anything outside the site's own /sites/foo/files directory results in an error.

I see you used /themes/custom/login-wallpaper.jpg. Can you try removing the leading slash and specifying a proper theme path? I tried values like: themes/custom/mytheme/logo.svg and theme/custom/mytheme/images/wallpaper.jpg respectively, and both worked just fine (note: no leading slash).

Sidenote, you may also be interested in the work on Random local images Needs work to specify a folder of random images.

🇪🇨Ecuador jwilson3

It seems like this may introduce discrepancy between the IMG tag used for the single default image versus the random wallpaper setting (handled by client-side js/wallpaper.js) which contains nothing more than the src and (empty) alt attributes.

🇪🇨Ecuador jwilson3

jwilson3 changed the visibility of the branch 3161447-2.0.x to hidden.

🇪🇨Ecuador jwilson3

I've restored the use_custom_wallpaper_folder field mentioned in comment #49 above, in order to fix comment #47 on the MR 37.

Comparing the diffs between both MR branches, they're now effectively the same.

You can confirm this by running the following commands:


wget https://git.drupalcode.org/project/gin_login/-/merge_requests/37.diff
wget https://git.drupalcode.org/project/gin_login/-/merge_requests/38.diff
diff 37.diff 38.diff

I'm pretty sure 2.1.x MR 37 will apply cleanly with fuzz against the 2.0.x branch, if needed.

IMO, 2.1.x branch MR 37 is ready to go.

🇪🇨Ecuador jwilson3

Regex changes look good. I've tested MR for the 2.1.x branch with a folder of JPGs and AVIF files. I tested invalid folder names. All works nicely. I made some minor grammar fixes to the form edit screen. Given the extra busy work involved in testing 2 branches, and cherry-picking commits between them, I don't see huge value in continuing the 2.0.x branch MR.

IMO, 2.1.x branch MR is ready to go.

Screenshots taken against the 2.1.x MR 37 with the dark variant of Gin admin theme.

🇪🇨Ecuador jwilson3

jwilson3 made their first commit to this issue’s fork.

🇪🇨Ecuador jwilson3

In addition to #39, I found the terminology of the form elements on the user edit form a bit confusing.

Currently, the UI looks like this:

› Admin theme settings
[ ] Enable overrides
    Enables default admin theme overrides.

I’d propose the following improvements:

  • Expose the field to Manage Form Display so it can be drag-n-dropped/enabled/disabled in different form modes. (comment #39)
  • Update the Details element label from “Admin theme settings” to “Administrative theme preferences”.
  • Revise the toggle field label from “Enable overrides” to “Personalize the appearance of the administrative UI”, and remove the redundant description text.
  • Implement a proper Field API widget with display options similar to the Address field widget, so users can select a Wrapper type (Container, Details, Fieldset) and override the default label text for Details/Fieldset.

The benefit of this approach is flexibility: if a frontend theme also provides customization options, both admin UI and frontend UI preferences could be grouped together under one field group. For example, by setting the wrapper to Container (invisible), you could present two checkboxes side by side:

› Website Look & Feel
[ ] Personalize the appearance of the backend administrative UI
[ ] Personalize the appearance of the frontend website UI
🇪🇨Ecuador jwilson3

I'm here to report a related issue I'm having whereby the "Admin theme settings" wrapper element gets added to every user edit form mode. I don't know if this makes sense to add here or not. This does have to do with "fine-grained control" over display admin theme user settings.

Steps to reproduce:

1.- Standard D10/11 site with Gin admin theme and "Enables default admin theme overrides" enabled.
2.- Go to /admin/config/people/accounts/form-display, expand "Custom display settings" and click "Manage Form Modes".
3.- Create a new User Form Mode call it anything you want, eg "Password reset" (machine name user.password_reset)
4.- Return to /admin/config/people/accounts/form-display and configure the new form mode to only display a single field: "User name and password"
5.- Visit /user/1/edit?display=password_reset
6.- See that the "Account" fieldset is visible, and the "Admin theme settings" details element is also shown below.

Proposed resolution:

Make "Admin theme settings" available for placement via drag and drop on Manage Form Display settings on /admin/config/people/accounts/form-display

If this can be tackled independently of the awesome overhaul going on in this issue, I'm happy to split this off to a separate issue, but I don't know enough about the plans here or whether it makes sense to roll this into this feature request or not. Thanks.

🇪🇨Ecuador jwilson3

Great reply. Thanks for taking time to clarifying your thinking and approach around generating and caching CSS files. That angle hadn’t occurred to me.

The only point I’d push back on is the dropdown implementation.

As it stands, the <button>-based environment switcher requires JavaScript to toggle aria-expanded between true and false. Without JavaScript, there are really only two HTML-native options: <select> or <details>.

<select> isn’t a good fit because many browsers don’t allow custom styling of option colors. Also it requires a <form> plus server-side handling and a redirect, when we only need simple links.

<details> works better because it allows us to maintain environment-specific colors in the links and avoids unnecessary form complexity.

That said, adopting <details> would be a fairly fundamental change—possibly significant enough to warrant a new major version bump.

It sounds like we have an architecture more or less hashed out here and could move forward with a POC.

🇪🇨Ecuador jwilson3

MR Ready for review

🇪🇨Ecuador jwilson3

Screenshots "after"

🇪🇨Ecuador jwilson3

Screenshots "before"

🇪🇨Ecuador jwilson3

jwilson3 created an issue.

🇪🇨Ecuador jwilson3

Remove all use of the [...] <script /> [...] elements.

If we remove use of the <script/> tag, how would the switcher dropdown functionality work? Maybe we can switch it to a highly stylized <details> element, instead of a <button which would require no additional scripts.

🇪🇨Ecuador jwilson3

Using style attribute for example shouldn't be necessary, when CSS rules don't need to be highly dynamic. Ie, background colors and text color etc is known, can be compiled to CSS files, and included formally.

Environment Indicator's foreground and background colors are highly dynamic, though. Historically, Environment Indicator requires project and environment-specific rules be be written in settings.php. (See code snippet on https://architecture.lullabot.com/adr/20210609-environment-indicator/ for one common pattern.). If we're not allowed to use inline styles via <style> tag or style="" attributes, how are we to supply the dynamic color changes per environment this module is used for?

The only thing that occurs to me is to create a Drupal route that dynamically generates a CSS file. But then that puts this module square in the realm of HTTP response caching, CSS/JS aggregation. and varying cache based on different project-based rules.

🇪🇨Ecuador jwilson3

It would be cool to feed three birds with one scone, and as part of overhauling color contrast issues, introduce css custom properties (aka variables, and come up with a color scale along the lines of either general named colors var(--webform-fg-gray-light) and combining them with more targeted, purpose-built attributes var(--webform-wizard-step-fg-color) and var(--webform-wizard-step-bg-color) (the third bird is that we'd be setting webforms up for native dark mode support).

🇪🇨Ecuador jwilson3

It is probably also useful to see when any Webform Nodes exist but do not reference any webform. A report going in both directions is akin to a more proper Correspondence or Bijection graph, where we're trying to ensure each entity has a matching partner, a natural 1-to-1 mapping.

The basic information can be obtained, roughly, with a simple query like this:

SELECT `webform`.`webform_id`, `node__webform`.`entity_id` as node_id FROM `webform`
LEFT JOIN `node__webform` ON `node__webform`.`webform_target_id` = `webform`.`webform_id`
ORDER BY node_id;

But this only accounts for webforms that either have or don't have Webform Nodes, it doesn't account for Webform Nodes that don't link to webforms, or Webforms referenced by multiple Webform Nodes.

For a more complete picture, we need a more complex query:

-- Show pairs + orphans + cardinality checks in one result.
SELECT
  x.webform_id,
  x.node_id,
  CASE
    WHEN x.webform_id IS NULL THEN '1_NODE_WITHOUT_WEBFORM'
    WHEN x.node_id   IS NULL THEN '2_UNREFERENCED_WEBFORM'
    WHEN wn.cnt_nodes_for_form = 1 AND nw.cnt_forms_for_node = 1
         THEN '5_PAIRED_1_TO_1'
    WHEN wn.cnt_nodes_for_form > 1 THEN '3_FORM_REFERENCED_BY_MULTIPLE_NODES'
    WHEN nw.cnt_forms_for_node > 1 THEN '4_NODE_REFERENCES_MULTIPLE_FORMS'
    ELSE 'CHECK'
  END AS status,
  wn.cnt_nodes_for_form,
  nw.cnt_forms_for_node
FROM (
  -- A) all webforms, with their referencing node if any
  SELECT w.webform_id, n.entity_id AS node_id
  FROM webform AS w
  LEFT JOIN node__webform AS n
    ON n.webform_target_id = w.webform_id
   AND n.deleted = 0
  /* optionally constrain to the node bundle that should carry the field:
     AND n.bundle = 'webform_node' */
  UNION
  -- C) nodes whose field points to a webform id that doesn't exist
  SELECT w.webform_id, n.entity_id AS node_id
  FROM node__webform AS n
  LEFT JOIN webform AS w
    ON w.webform_id = n.webform_target_id
  WHERE w.webform_id IS NULL
    AND n.deleted = 0
  /* optionally: AND n.bundle = 'webform_node' */
) AS x
LEFT JOIN (
  -- count how many nodes reference each webform
  SELECT webform_target_id, COUNT(*) AS cnt_nodes_for_form
  FROM node__webform
  WHERE deleted = 0
  GROUP BY webform_target_id
) AS wn
  ON wn.webform_target_id = x.webform_id
LEFT JOIN (
  -- count how many webforms each node references (should be 0 or 1)
  SELECT entity_id, COUNT(*) AS cnt_forms_for_node
  FROM node__webform
  WHERE deleted = 0
  GROUP BY entity_id
) AS nw
  ON nw.entity_id = x.node_id
ORDER BY status, x.node_id, x.webform_id;
🇪🇨Ecuador jwilson3

Since we're removing email due to limitations above, it makes sense to also not use 'Co-authored-by' — in addition to the other arguments made above — because other platforms would typically expects a username + email in that format.

In #23 my proposal was that if consensus is that we're not trying to satisfy machines and us a standard format, then I'd propose taking it one single step further than just the technical email limitations written in the issue summary, and make the 'By: ' trailer readable as a single comma-separated list of usernames, which aligns more closely with how we used to do it.

Before:

Issue #999999 by user1, user2, user3, user4, user5, user6, user7, user8, user9: Convert MediaSource plugin discovery to attributes

After:

[#999999] task: Convert MediaSource plugin discovery to attributes

By: user1, user2, user3, user4, user5, user6, user7, user8, user9

Instead of:

[#999999] task: Convert MediaSource plugin discovery to attributes

By: user1
By: user2
By: user3
By: user4
By: user5
By: user6
By: user7
By: user8
By: user9

I didn't see anyone address this point yet and it was the primary point I was trying to make. Sorry for what sounds like it devolved into a bikeshed, but this goes directly to the point of considering how the data is used so we can get the format right once and not change it again for 10+ years.

🇪🇨Ecuador jwilson3

I'm surprised no one has mentioned wildcard support here.

webform_submission_*

Were this MR to have wildcard support, then we could likely close other 3rd party module integration issues like Honeypot Facets 3 submodule? Active and Add "Protect all Webforms" options to Honeypot's global module config Active . Additionally, 🐛 Conflicting ID when honeypot is rendered more than once on a page Active could potentially even be solved via wildcard support as well.

The honeypot module already has a dev dependency on Webforms and tests that enables webform, so we could use that module in a wildcard test on the MR here.

🇪🇨Ecuador jwilson3

As @longwave pointed out in previous comments, GitLab, GitHub and others seem to have come to some level of semi-standardization around the Co-authored-by: username <email> trailer, though this is not official in any way. So if the goal is for getting off the island, I'd go with that. On the other hand, if the goal is to just make the old "byline" info in the git commit message available (and easily human-scannable) for casual users, then a custom By: [username1]\nBy: [username2] is okay and By: [comma-separated list of users] even better, since IDE tools that do inline Git blame popups, like GitLense or Better Git Line Blame (which I use in Cursor) would get vertically truncated for really long multi-line commit messages, making it less scannable.

I think it comes down to what problem we're trying to solve. Since we use GitLab, it seems smart to make the effort and kill 2 birds with 1 stone by going with something that GitLab supports. https://github.blog/news-insights/product-news/commit-together-with-co-a...

From https://gitlab.com/gitlab-org/gitlab-foss/-/issues/31640

Gitlab should recognize Co-Authored-By: [name] <[email]> in the message body, and should display user icons on UI related to the commit

they don't actually link to the user from the commit message, as is suggested here, but they list the co-authors along with the main author in the interface

🇪🇨Ecuador jwilson3

Follow! My understanding is that AI bots are not actually submitting forms, but that the bots love clicking through links and the Facets issue boils down to the fact that Facet blocks aren't really checkboxes, but are in fact just links styled to look like checkboxes. Facets 3 switches to using actual checkboxes and forms instead of lists of links, which is promising and will probably save us for now.

But as time wears on and bots get more advanced, there might be real opportunity here. But my suggestion would be to first try adding additional form ids for the Facets 3 block forms to the existing Honeypot module settings on your site, to guage whether Honeypot + Facets forms it is having any effect in rebuffing bot requests. Allow for more flexible form configuration Active seems like a promising solution, if you're not opposed to manually entering facet form ids. If this does help, then maybe adding a UI option to manually enter custom form ids would be a good first step. A more custom UI just for Facet blocks could be possible, but probably complex, and easily doable from a few lines of custom form_alter code.

Here is what I'm doing to add Honeypot to all Webforms. Presumably it could be adapted for Facet blocks as well assuming they follow some kind of standard form id prefix in their naming conventions.


function mymodule_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  // Add honeypot protection to all Webforms.
  // @todo remove when https://drupal.org/i/3544510 lands
  if (strpos($form_id, 'webform_submission') === 0) {
    if (\Drupal::moduleHandler()->moduleExists('honeypot')) {
      // There is no reason to not enable time restriction here since
      // Honeypot respects the time_limit set in the configuration.
      \Drupal::service('honeypot')->addFormProtection($form, $form_state, ['honeypot', 'time_restriction']);
    }
  }
🇪🇨Ecuador jwilson3

The form-specific "Protect [FORMNAME] webform with Honeypot" feature is provided by the Webform module, so we'll need a follow-up created in the Webform module issue queue to respect the value of the honeypot module's global setting, and disable the per-form option accordingly.

🇪🇨Ecuador jwilson3

Okay, that all makes sense, but now I'm trying to understand whether the (fallible, potentially inaccurate) author credits in the commit message is intended to serve machines or humans?

🇪🇨Ecuador jwilson3

I still prefer the simplicity and less wordy By.

If an issue has 20+ authors, then a newline and By: ends up getting really long itself. If we're back to only showing username instead of username <email>, then maybe we can also go back to a single line format like: By: author1, author2, author3, author4.

🇪🇨Ecuador jwilson3

I've come here via a link to this issue, currently buried inside a collapsed FAQ item on the new contrib record page (https://new.drupal.org/contribution-record/:id ), which is currently being encouraged to module maintainers.

If you are committing, you are encouraged to use the formatting here.

Is it really safe to recommend people start using a new commit message format if the part about multiple authors is not yet settled? Will issue credit be properly picked up, if the format ends up changing?

🇪🇨Ecuador jwilson3

Thank you for creating this issue to improve the Shortcut per Role module.

I am working through modules I help maintain to decide if this task is still relevant to the currently supported version of the module. There hasn't been any follow-up here for over 9 years which suggests that this is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information as to whether this issue still applies, it may be closed after three months.

Thanks!

🇪🇨Ecuador jwilson3

Thank you for creating this issue to improve the Shortcut per Role module.

I am working through modules I help maintain to decide if this task is still relevant to the currently supported version of the module. There hasn't been any follow-up here for over 9 years which suggests that this is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information as to whether this issue still applies, it may be closed after three months.

Thanks!

🇪🇨Ecuador jwilson3

Thank you for creating this issue to improve the Shortcut per Role module.

I am working through modules I help maintain to decide if this task is still relevant to the currently supported version of the module. There hasn't been any discussion here for over 9 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

🇪🇨Ecuador jwilson3
  • @drupal_infra on Twitter/X - last updated 3 months ago, lately only shows planned outages, has intuitive threading (first report shown first, follow-ups threaded underneath).
  • @drupalinfra on Mastodon - last updated 3 days ago, shows both planned and unplanned outages, has an unintuitive threading (follow-ups show up first, hard to find the original incident report).
  • #drupal-infrastructure on Slack - last updated 12 hours ago, shows early incident reports, direct support from infra maintainers, is intuitively threaded (first report shown first, follow-ups threaded within).

Having a third-party hosted incident report / status page would be super nice, and there are plenty to choose from:

But there are several issues:

  • There are too many platforms to choose from, leading to bikesheding (see: this issue).
  • Ultimately, the implementation requires assessment and buy in from maintainers, which takes even more time.
  • Implementing an entirely new channel just to replace the embed on the 500 error page seems like the wrong angle to come at the problem.
  • A new dedicated channel will upset existing and expected outage workflows for both community users and maintainers.
  • Setting the embed aside, it seems that most bases are covered with Mastodon and Slack, so introducing yet another channel adds noise — not to mention more busywork for Infra team to post updates across channels — unless steps are also taken to pull back from all the other platforms and centralize comms.

Reframing this problem from an end user perspective, when I see a 500 Error page the first thing I want to do is to be able to see if the incident is known, and if not know where to go to report it. I typically go to the https://x.com/drupal_infra first but only because that's where I'm directed to go. If I don't see anything reported there — it usually hasn't been — then I head over to #drupal-infrastructure channel on Drupal Slack to check for a report or discussion — it usually has been. From there, I can then finally see the link in the channel description to https://mastodon.social/@drupalinfra which might have different outage information than what is reported in Slack.

As an interim solution, could we just point to Mastodon and (optionally, the Drupal Slack channel) instead of Twitter/X on the 500 error page? Do we really even need to replace the embed on the 500 page?

🇪🇨Ecuador jwilson3

Thanks @aficiona2 for catching the math error. a 5kb image is way more reasonable for inlining with data URI.

Here are some things we can try to reduce the LQIP-LCP image size even further:

1. Lower the WebP quality parameter to its minimum (quality=1). However, in my testing, even at this setting, the generated file size is still much larger than the theoretical minimum bits-per-pixel limit.

  • Full-res image: ~40 KB
  • Theoretical LQIP-LCP image: ~5 KB
  • Actual LQIP with WebP configured to use quality=1: ~14 KB

Other potential approaches:

2. Downscale the image before converting to WebP at quality=1.

3. Apply a blur (box or Gaussian, with varying strengths) before converting to WebP at quality=1.

4. Downscale, then upscale the image before converting to WebP at quality=1.

5. Downscale, convert to WebP at quality=1, then upscale client-side (in the browser).

6. Experiment with other lossy formats/algorithms (e.g., JPEG, AVIF). However, some formats may not achieve smaller sizes than WebP at their lowest quality settings.

It’s worth noting that achieving the theoretical bits-per-pixel limit is highly dependent on the visual complexity and entropy in the source image. Simple images compress much better than complex, detailed ones. To find a reliable algorithm or processing pipeline that consistently approaches this limit across a variety of images and sizes will likely be more challenging than it appears, and may require a multi-pass approach (adding to algorithmic time). Extensive testing with diverse images will be necessary to identify a robust solution. For that reason, I humbly also add a 7th option:

7. Look to see if someone has already worked on an "LCP" algorithm to do this already.

🇪🇨Ecuador jwilson3

Why not use a single DDEV environment for a core branch and clone contrib projects into it?

Two main reasons:

  • DDEV environments are cheap to spin up. It’s better to isolate contrib modules in their own environments to avoid conflicts and ensure clean, reproducible setups. Sharing a single site across unrelated modules can create unnecessary complexity around config, database, and dependency management.
  • Composer isn’t great at working from local composer.json files across multiple contrib modules. It typically tries to fetch packages from the web, making it hard to test local changes to a module’s dependencies. By contrast, ddev-drupal-contrib builds directly from a module’s local composer.json, including require-dev dependencies for optional integrations.

This approach avoids local dependency tree hell and ensures all contributors run the same clean, isolated, and predictable environment.

🇪🇨Ecuador jwilson3

t would be nice if there was an automated solution to always minify SVGs in core in the same way, because different tools optimize in different ways, using different default options, often stripping out important attributes. Applications that generate SVGs also generate their own different ways of structuring the SVG, so every time you pull an "optimized" file into an SVG editor, and save it again, it may make unrelated changes on the roundtrip back to optimized SVG. Therefore, the SVGs require manual edits after optimization to restore settings.

I think what is missing here is clear agreement to treat SVGs as implementation assets, not hand-curated visual code.

🇪🇨Ecuador jwilson3

Also worth noting: running SVGs through optimizers like SVGO can make them harder to review and maintain. For example,

path="M11 0L14 3V16H2V0H11ZM3 15H13V4H10V1H3V15ZM11 3H12.5L11 1.5V3Z"

becomes

path="m11 0 3 3v13H2V0zM3 15h10V4h-3V1H3zm8-12h1.5L11 1.5z",

which is more compact but much less readable for quick diffs or manual review.

🇪🇨Ecuador jwilson3

Isn't this kind of micro-optimization mostly obviated by gzip or brotli? I'm happy to be wrong, but these savings seem negligible in practice—especially considering that Drupal core’s own HTML output doesn’t aggressively strip whitespace, newlines, or indentation either.

🇪🇨Ecuador jwilson3

Here is a partial config export for what I've tested:

    ckeditor5_paste_filter_pasteFilter:
      enabled: true
      filters:
        -
          enabled: true
          weight: -18
          search: '<o:p><\/o:p>'
          replace: ''
        -
          enabled: true
          weight: -17
          search: '(<[^>]*) (style="[^"]*")'
          replace: $1
        -
          enabled: true
          weight: -16
          search: '(<[^>]*) (face="[^"]*")'
          replace: $1
        -
          enabled: true
          weight: -15
          search: '(<[^>]*) (class="[^"]*")'
          replace: $1
        -
          enabled: true
          weight: -14
          search: '(<[^>]*) (valign="[^"]*")'
          replace: $1
        -
          enabled: true
          weight: -12
          search: '<font[^>]*>'
          replace: ''
        -
          enabled: true
          weight: -11
          search: '<\/font>'
          replace: ''
        -
          enabled: true
          weight: -10
          search: '<span[^>]*>'
          replace: ''
        -
          enabled: true
          weight: -9
          search: '<\/span>'
          replace: ''
        -
          enabled: true
          weight: -8
          search: '<p>&nbsp;<\/p>'
          replace: ''
        -
          enabled: true
          weight: -7
          search: '<p><\/p>'
          replace: ''
        -
          enabled: true
          weight: -6
          search: '<b><\/b>'
          replace: ''
        -
          enabled: true
          weight: -5
          search: '<i><\/i>'
          replace: ''
        -
          enabled: true
          weight: -4
          search: '<a name="OLE_LINK[^"]*">(.*?)<\/a>'
          replace: $1
        -
          enabled: true
          weight: -3
          search: '<p>\W*<br>\W*&nbsp;\W*<\/p>'
          replace: ''
        -
          enabled: true
          weight: -2
          search: '<p>\W*<br>\W*<\/p>'
          replace: ''
        -
          enabled: true
          weight: -13
          search: '(<[^>]*) (dir="ltr")'
          replace: $1
        -
          enabled: true
          weight: -1
          search: (<br>)+
          replace: '<br>'
        -
          enabled: true
          weight: 0
          search: (<p><br></p>)+
          replace: '<p><br></p>'
        -
          enabled: true
          weight: 1
          search: '<br>\n'
          replace: '<br>'
        -
          enabled: true
          weight: 2
          search: \n+
          replace: '<br>'
🇪🇨Ecuador jwilson3

I'm not certain if I should change the $use_case to a new number

The use_case needs to be incremented to the lowest unused number. (Currently that would be 18).

🇪🇨Ecuador jwilson3

Maintainer here. Thank you for the excellently written issue complete with MR!

I get the sense from the resulting markup that _label_help_append_title() is not ideal from an accessibility standpoint

You're probably right here. _label_help_append_title() is kind of the "worst case" scenario when nothing else works.

So what I recommend you do is to replace that with the other options to see if any of the other options work better and put the label help in the correct place, visually, on the form element.

_label_help_prepend_field_prefix($element, $content, $use_case);
_label_help_prepend_description($element, $content, $use_case);
_label_help_append_label_suffix($element, $content, $use_case);

If any of these work better than append_title, pls update to the MR. If not, leave a comment to that effect.

Thank you!

🇪🇨Ecuador jwilson3

I added ddev and ddev-drupal-contrib to Svg Image Field contrib module and to Rivet contrib theme, but ddev-drupal-contrib advocates for also committing the custom ddev commands it provides, it does become burdensome to keep things updated as the drop keeps moving (between ddev updates, drupal updates, ddev-drupal-contrib updates).

instead of putting a complete ddev config inside each module, a single config file should be sufficient.

Ideally, in the future, boilerplate code for providing local development environments will be reduced.

Strong 1+. This seems like the ideal goal to shoot for. For example, it would be fantastic if ddev addons (like ddev-drupal-contrib) could be added to a single .ddev/config.contrib.yml file and not have to commit all those custom commands. Users could run ddev config for their environment, and ddev pulls in the contrib file. Configuration overrides & customizations would go in .ddev/config.local.yml. Then, if the contrib module or theme itself needs custom DDEV commands, docker settings(?), or other things it could either commit them directly to the module's .ddev folder or create a separate ddev-addon-template, and add a dependency for their own ddev addon maintained in a separate codebase.

🇪🇨Ecuador jwilson3

I added a way to run lighthouse (locally, via the npm library) against all of the pages in the test site. I ran against the pages using a 1s delay and without any delay at all. Each run gives slightly different performance score but generally stays within a 2 point range in the mid nineties on each page, except for one. The "Ultimate LQIP" page can vary between a score of 100 AND then go consistently below 90 (as low as 87) in different runs. This must have something to do with the extra intermediate low-res image latency being what amounts to a double edged sword.

https://3523781-drupal-lqip.elementalidad.com/results.php

https://github.com/jameswilson/3523781-Drupal-LQIP/commit/245cbbc1d787a2...

🇪🇨Ecuador jwilson3

Any chance we could get a 3.1 (or 3.0.1) release that includes this fix? This generates a lot of unnecessary noise in logs.

We are seeing deprecation warnings on cron run after updating to PHP 8.3.21 and clearing caches. These are only notice errors, I think, so we should still be fine. But there are about 500 of them after running cron from the admin UI.

    Deprecated function: Creation of dynamic property 
    Drupal\feeds\Feeds\Item\SyndicationItem::$blog 
    is deprecated in Drupal\feeds\Feeds\Item\BaseItem->set() 
    (line 21 of modules/contrib/feeds/src/Feeds/Item/BaseItem.php).

    Deprecated function: Creation of dynamic property 
    Drupal\feeds\Feeds\Item\SyndicationItem::$parent:field_group 
    is deprecated in Drupal\Core\Queue\DatabaseQueue->claimItem() 
    (line 150 of core/lib/Drupal/Core/Queue/DatabaseQueue.php).
🇪🇨Ecuador jwilson3

Re: #15

adding the css .loaded class in inline js with a CSS rule is causing it to be the LCP again. So I don't think it's showing what the LCP would be for the different approaches.
  • The .loaded class is used on several of the examples (SQIP, BlurHash, Ultimate LQIP, CSS-only LQIP) to change opacity with a smooth transition between the blurred placeholder and the full-res version.
  • For contrast, the LQIP WebP Smooth example uses a slightly different technique to change opacity smoothly, via an inline style opacity:1 loaded via inline JS onload, but the smoothing transition effect on the opacity is still defined in CSS (I don't believe the distinction between inline vs CSS ultimately matters for performance other than if your page has many images at some point you're sending down a lot of duplicitous bytes).
  • Finally, the basic examples (LQIP BMP, LQIP PNG, and LQIP WEBP) do not use any smoothing transition and just rely on browser loading to show the full-res image and overlay the low-res placeholder.

I don't think LCP is affected by the opacity change (both with or without smoothing effect). Rather, the problem with the LCP is that all examples except "Ultimate LQIP" use placeholders that have a Bits-Per-Pixel ratio far less than the recommended 0.05 which means LCP will always consider the repaint of the full-res image.

The takeaway is that we cannot effectively reduce LCP with the LQIP technique unless the placeholder image is large enough >0.05BPP. And for the image to be "large enough" to take over the LCP for the 1200 pixel wide hero in the example site, it has to be on the order of 40k in size, which IMO rules out the option of base64 inlining. This implies the LQIP must be a reference to another image file, and yet another request (with potential latency) to download the placeholder image, somewhat defeating the other intended of the goal of the LQIP (having something on screen fast, at page load time, ideally piggy-backed inline via the HTML request).

🇪🇨Ecuador jwilson3

The data model and schema should support all fetchpriority values — no question there. But I’d question the utility of exposing them at the image field formatter level.

Take the carousel example: there are (at least) two common patterns in Drupal:

  1. A multi-cardinality image field used as the image source, or
  2. A series of entities (e.g. paragraphs, ECK) each with a single image field.

In both cases, there's no reliable context at the field formatter or view mode level to determine which images are visible at page load. Exposing fetch priority there risks encouraging awkward architectures — e.g., splitting fields just to differentiate high vs low.

The complexity only increases with variations like random first slides or responsive carousels showing N slides. Handling these correctly almost always requires custom logic (alter hooks, preprocess).

A UI trying to cover this would have to offer nuanced options like:

  • Unspecified (let browser decide)
  • High (for LCP-critical images)
  • Low (for images above the fold but not visible initially)

For multi-cardinality:

  • First is high, others unspecified
  • First is high, others low
  • First N are high, rest low/unspecified

…which quickly gets unwieldy and still doesn’t address entity-based carousels.

In short, this might be better solved downstream (e.g., in preprocess) where real context exists.

🇪🇨Ecuador jwilson3

Re: #15 (.loading class LCP issue) Thank you. I'll try to make sense of what you're saying and get to the bottom of this soon. But happy to have a PR if you know what the fix would be offhand. (The project is setup for DDEV running locally).

Re: #16 (SQIP) While SQIP is a "superior" image, there are two major reasons it is problematic: it is both resource intensive on the server-side and on the client-side. For the server-side, even if we were to reimplement `sqip` in PHP, I expect it will be a fair bit more resource intensive than simply scaling down a raster image to 8x8 and applying a simple box blur, which Drupal image styles can do for us OOTB today. On the client-side, the problem is the <g filter="blur(12px)"> which is applied via browser rendering. you can inspect https://3523781-drupal-lqip.elementalidad.com/images/hero.sqip.svg to see the file that was generated by the npm library.

The sqip command took about 2.6 seconds to run, used about 4 CPU cores, (thats >10s total compute time) for the 400kb image.

If you look at the animated GIF demonstrating the processing progression of the underlying binary used by sqip, it becomes fairly obvious this is intensive work: https://github.com/fogleman/primitive?tab=readme-ov-file#progression

🇪🇨Ecuador jwilson3

For base64 images we need img-src data:, not entirely sure about SVG.

If you load SVG via data URI (e.g., src="data:image/svg+xml;base64,..."), then it would be covered by img-src data:. But we'll also need to ensure any generated SVGs (especially via 3rd parties) are additionally XSS sanitized.

It's a good point to consider, and maybe we'd have to have a site-level configuration to pick using inline data uris, or additional requests for the thumbnails.

🇪🇨Ecuador jwilson3

I set up a test site with a few LQUIP approaches to be able to test the visual load transitions (using a poor man's "delay" dropdown parameter to simulate latency and to actually see the LQIP for more than a brief second).

  1. A couple baselines (OOTB Drupal eager/lazy load settings).
  2. A couple basic LQIPs based on an inline square 8x8 thumbnail inspired by how Unsplash does it. Unsplash uses an inline BMP, but I also tested with inline PNG (same size as BMP), and an inline WebP which produced a much smaller inline payload as well as a slightly different visual blur (the BMP and PNG were visually equivalent). The key here is to add a simple box blur to the 8x8 thumbnail to avoid browsers rendering jagged edges between adjacent high-contrast pixels when scaling up the thumbnail to full-res size in the browser. I also tested without blur, and with larger thumbnails like 16x9, but none of these options look as visually appealing as the simple, blurred 8x8 square image.
  3. A LQIP WebP Smooth using an 8x8 blurred WebP inline thumbnail with smooth fade-in effect requiring an "onload" JS to transition from low- to high-res. IMO this is the clear winner to satisfy the visual perspective, architectural simplicity (no 3rd party deps aside from GD), and resource usage both client- and server-side.
  4. The Ultimate LQIP technique, which depends on 2 LQIPs and suffers from the twice the number of http requests.
  5. The Blurhash technique. Blurhas has an existing Drupal module, but calculating the blurhash is fairly resource intensive on the server-side, and the Drupal module doesn't have any caching. It also depends on the clever base83 hash being decoded with Javascript on the client-side, but the 3rd party library is a JS module which complicates usage for Drupal requiring the use of import and knowing the path to the library JS file.
  6. A couple CSS-blur techniques including:
    • a client-side blur of a small thumbnail. CSS blur applied to an image looks really bad around the edges of the image and is a non-starter.
    • CSS-only LQIP technique. This has horrible CSS complexity to create the integer hash and the clien-side CSS gradient code to "decode" the integer hash. Also, the resulting effect of the grayscale gradient applied onto the image's calculated average color (which must be calculated server-side from the source image) look extremely simplistic compared to the 90-byte WebP thumbnail.

I had a look at LCP for each of these with WebPageTest and PageSpeed Insights, but couldn't find a solid winner emerge. In my tests, the "Ultimate LQIP" option had the worst LCP of them all on WebPageTest, so it appears that the LCP algorithm may vary based on what tool is being used. YMMV.

Ultimately, I think the LCP goal is possible but difficult to achieve for hero images with an LQIP approach alone, since you need twice the requests and the >BPP0.055 ratio ends up creating a fairly large image for the LCP (44-kilobyte) versus a simple 90-byte 8x8 thumbnail. However, it is also worth noting that none of the other approaches I've found that depend on a low-res blurred image or css-gradient will positively affect LCP since they inherently do not meet the minimum BPP ratio.

Looking forward to having others' thoughts, insights, and reviews.

Code here: https://github.com/jameswilson/3523781-Drupal-LQIP
Site here: https://3523781-drupal-lqip.elementalidad.com/

🇪🇨Ecuador jwilson3

Because the blurhash is calculated inside a hook_preprocess_image() function, unnecessary and costly regeneration of the hash happens after a Drupal cache rebuild or anytime the hook is executed such as during theme development when Twig caching is disabled. This severely hinders the theming process so as to be completely unusable on any page with an image of significant size.

Therefore, I would reiterate the importance of caching the generated blurhash either in the database (as issue summary suggests) or at the very least in the filesystem.

🇪🇨Ecuador jwilson3

While the server-side blur is probably feasible, I don't think this precludes us considering tiny inline images (smaller than 20x20px).

Maybe next steps could be to create a few simple HTML page examples using the different approaches to see how Lighthouse LCP behaves.

Proposed tests:

  • Baseline 1: JPG Hero image with lazy loading (Drupal's default option for all images).
  • Baseline 2: JPG Hero image with eager loading.
  • LQIP w/o CSS blur: Hero image with basic LQIP technique using a tiny base64 inline WebP placeholder image (a scaled down version of the hero, sizes to no more than 20×20 pixels).
  • LQIP w/ CSS blur: Hero image with LQIP technique using a tiny base64 inline WebP placeholder image with client-side blur applied.
  • SQIP
  • Pure CSS LQIP.

Points to consider for fair comparison:

  1. payload size: inline WebP base64 versus inline SVG versus CSS (+ JS) size.
  2. placeholder render quality: a CSS blur will look good but could the <20×20px LQIP base64 payload image with no blur applied work (for reduced client-side processor power and shaving bytes off CSS)?
  3. load experience: is there any jank between the the visual shift from placeholder to full-res image? is it annoying enough to need a JS fade-in effect?
  4. Largest Contentful Paint score: Does each technique provide an efficient Lighthouse' LCP based on the placeholder, or do any of them get a longer LCP due to the full-res image loading in later?
  5. processing requirements: This will be hardest to confirm outright, but a tiny inline image with no CSS blur and minimal to no JS is more efficient than more complex solutions, and should count for something unless any of the previous points disqualify it.
🇪🇨Ecuador jwilson3

I've rebased the MR on the latest 11.x in an attempt to clear unrelated CI failures...

🇪🇨Ecuador jwilson3

Why not just inherit the one from Drupal core?

Production build 0.71.5 2024