🇺🇸United States @Luke.Leber

You shouldn't need to do anything with the regex deprecation :-). That'll work in D11.

That deprecation message is one that I added to let any users that are customizing this module with their code that some interfaces will be changing in 3.0.x (which I don't have a timeline for yet).

I'm hoping to get a release cut this week with d11 support.

Yes - thank you. The call to `::getEmbedCode` is an internal deprecation to this module. It looks like there are some automated test fails in HEAD that need to be addressed, but this module is on my task list this week to bring up to speed and get the issue queue flushed.

I can review the commits since last release and cut a new one when I get back from vacation next week if another maintainer doesn't beat me to it. 👍

Agree that the ship has likely sailed with such naming convention standardizations as the landscape exists.

The time to decide such things was before there were literally billions of records and tens of thousands of custom sites depending on certain field names.

Any changes at this point would likely require a brand new way to performantly migrate huge volumes of records to new columns and would likely require maintaining redundancies between multiple major Drupal versions to prevent disastrous upgrades.

Luke.Leber → created an issue.

griffynh → credited Luke.Leber → .

#6 / #7 do not appear to resolve things holistically.

I still maintain that addressing this should happen in the webform_clientside_validation module.

Adding missing issue credits from security.drupal.org

This one is interesting. Can you please provide the custom form source that you encountered this error on?

Typically bug fixes must include automated test coverage to ensure the same type of bug can't slip back in unnoticed in the future 🙏.

I am mobile now and had a bit this evening to myself. Trying to move this issue along.

I've simply moved the ES6 asset in place of the ES5 and deleted the build stuff. Hope to be able to test against a real site soon, unless someone beats me to it 🙏.

Hey all, after thinking a bit about all of this I've transferred issue credits to https://www.drupal.org/project/media_library_form_element/issues/3458296 📌 Update build dependencies to work with a supported Node version Active 👍. I don't see any valid reason to continue running a node based build step for this module.

If there are any alternative views or there were any missed parties in the issue credit transfer, please reply to #3458296 🙏.

Next major failure is to be expected as a peer test-only dependency on Webform cannot be fulfilled.

The next-major failure shouldn't stop this issue, nor a D11 compatible release, as the Webform incompatibility will be managed by Composer. In theory, as soon as a Webform D11 compatible release is made, the NEXT_MAJOR test here should start passing automatically (so long as we fix the issue with the current LTS version of Drupal!)😁.

Luke.Leber → created an issue.

Transferring issue credits from https://www.drupal.org/project/media_library_form_element/issues/3432301 📌 Request to remove build dependencies in stable releases Needs review

Ship it.

Looks good to me; let's get this pulled into a live site and continue alpha testing.

It looks like perhaps an HTML table is needed in the markup to be diffed here? Can you please provide the content you're seeing this on? 🤞

We'll definitely need a failing automated test before committing any sort of bug fix to this module.

👋 diff 1.1.0 is now released.

If you run into any issues please don't hesitate to open another issue.

Thanks for nudging me into actually getting that done. 😅

Pushed to 1.0.x - thanks!

Good morning, I'm hoping to cut a new release this weekend with diff 2.x support. There's another issue that's RTBC in the queue that's ready to merge I feel 👍

We've started to use drupalSettings + js_settings_alter to provide server-side rendered templates to the frontend.

Our Drupal.theme.messages implementation pulls the markup from drupalSettings. Might not be robust enough for core, but has worked rather well for us...and even covers the obscure file.module validation cruft too.

Seeing as https://www.drupal.org/project/webform/issues/3460222#comment-15692042 📌 Consider deprecating Choices Closed: duplicate closed a related issue as a duplicate, I wanted to bring over the opinion that deprecating, discouraging the use of Choices, and ultimately removing it from Webform seems to be the most responsible path forward here.

The library is abandonware.

Agreed! This is actually a core issue I believe.

Added upstream issue and marked postponed 👍.

Luke.Leber → changed the visibility of the branch 10.1.x to hidden.

re: #22 - It looks like the patch from #5 was made against Drupal 9 (which shipped with *.es6.js files transpiled to es5). #20 seems to be for Drupal ^10.

MR !3355 just straight-up removes the javascript, which isn't a valid approach IMO.

I feel this should be addressed and have test coverage added in a follow-up. Javascript based testing like this is very hard relative to the simplicity of the fix.

Overriding constructors isn't recommended when the parent class implements the create method, as constructors aren't covered by B/C. Leaving as NW.

Please feel free to re-open if you are still experiencing issues after going through the docs :-).

Maybe next time, update bot.

All green on ^10.2 || ^11. Given the automated test coverage, and lack of non-test changes here, I feel it's safe enough to merge as-is.

Cumulative release testing must happen before a new stable is cut.

Luke.Leber → created an issue.

Injecting here that this would make styling file upload errors quite a bit easier!

It'd pay to take a hard look at the file module to determine how feasible the approach in the IS actually is.

Temp patch for 10.3.x that prevents a silly

1. Site is slow
2. Site is fast (after code deployed, before updb runs)
3. Site is slow (after updb runs, before config imports)
4. Site is fast (after config import turns the module back off

deployment strategy 🤭.

Intended use is to patch only during the 10.3 upgrade then remove. This can be helpful for users that consistently see database deadlock problems arising from huge block plugin cache writes...

diff --git a/core/modules/layout_builder/layout_builder.post_update.php b/core/modules/layout_builder/layout_builder.post_update.php
index ee735bf41b..6fd8891218 100644
--- a/core/modules/layout_builder/layout_builder.post_update.php
+++ b/core/modules/layout_builder/layout_builder.post_update.php
@@ -74,5 +74,6 @@ function layout_builder_post_update_timestamp_formatter(?array &$sandbox = NULL)
  * Enable the expose all fields feature flag module.
  */
 function layout_builder_post_update_enable_expose_field_block_feature_flag(): void {
-  \Drupal::service('module_installer')->install(['layout_builder_expose_all_field_blocks']);
+  // It's silly to install this only to have config import uninstall it.
+  // \Drupal::service('module_installer')->install(['layout_builder_expose_all_field_blocks']);
 }

Telephone numbers are very complex. A simple input mask isn't a reliable mechanism here, as number formats do vary wildly from country to country.

There's an international telephone mode feature for this that ships with country-specific masking capabilities 👍.

I can confirm some general weirdness with the i18n phone feature when used in conjunction with client side_validation.

It seems like both clientside_validation's and webform's JavaScript is both running against it and that can lead to some confusion when trying to theme things (the error messages can appear in different places in the DOM based on which validator "wins".

Luke.Leber → created an issue.

I'm not 100% sure if Drupal/yml marks a distinction between double quotes vs single quotes in yml unless there are escape characters involved. Might be something Wim or the config validation folks could answer pretty easily as they're the config wizards 😆.

Woo!

Indeed it has; adjusted the composer requirement...let's see what the test runner thinks.

The MR looks great; I'll try to get this into a stable d11-compatible release this week after some manual testing. Sorry it's taken this long.

Luke.Leber → made their first commit to this issue’s fork.

Savvy users can also utilize composer to further harden their applications:

Example: Nuke the vulnerable file from orbit on `composer install`

    "scripts": {
        "drupal-scaffold": "DrupalComposer\\DrupalScaffold\\Plugin::scaffold",
        "post-install-cmd": [
          "rm docroot/libraries/choices/public/index.html"
        ]
    }

🛡️

Just chiming in here from a different perspective: our group has disallowed the use of Choices.js for years due to inherent accessibility issues.

Given that...

1. The software hasn't had a commit in ~3 years
2. The software ships with a vulnerability
3. The Select2 implementation ranked higher in [our] a11y testing

Might it be an alternative to deprecate / disable its use in Webform by default? Shipping what is effectively abandonware doesn't seem sustainable.

Thanks Mark. I'll ping you in edudu on next steps for release.

Postponed on https://www.drupal.org/project/media_library_form_element/issues/3432301 📌 Request to remove build dependencies in stable releases Needs review

Opened drupal.org/project/media_library_form_element/issues/3457129 as a follow-up; re-closing :-)

Luke.Leber → created an issue.

Following up here - this is what I get on Drupal 10.1 when attempting to run a drush cr on a site with 2.x:

www-data@5bae608a62be:~/html$ drush cr

In YamlFileLoader.php line 161:
                                                                                                                                                                                                                          
  The configuration key "autoconfigure" cannot be used to define a default value in "modules/contrib/media_library_form_element/media_library_form_element.services.yml". Allowed keys are "public", "tags", "autowire".  

I think we may have to declare ^10.3 only here?

Opened https://www.drupal.org/project/media_library_form_element/issues/3458296 📌 Update build dependencies to work with a supported Node version Active as a follow-up for a better long-term solution.

Luke.Leber → created an issue.

I've opened !13 against this issue, but it's not a good solution (imo).

I had to downgrade all the way to Node 10 to get this project to build. While !13 is likely better than HEAD, this project should be upgraded to work with Node 20 or later; perhaps in a follow-up.

Hey @millerrs - we're working toward a 2.1.0 release. There's an ongoing discussion right now that has to be resolved first.

I think we might need a follow-up to update the Choices library after https://github.com/Choices-js/Choices/pull/1162 lands upstream (Thanks
Jürgen!)

Presently, the choices.js library, as pulled in via `composer.libraries.json` will continue to place `./[web-root]/libraries/choices/public/index.html` on disk which loads scripts from polyfill.io. Given sophisticated enough malicious javascript, this file could potentially be used by threat actors to launch phishing attacks.

It looks like the change was originally reverted on 6.2.x back on April 23, but wasn't on 6.x until June 24. Seems 6.x and 6.2.x are kept somewhat in sync.

Timing on !5500 on 19621 blocks, 24290 revisions, and a batch size of 50 (default).

real    0m19.932s
user    0m11.936s
sys     0m0.955s

~20 seconds is a lot more acceptable than 30 minutes! What a difference the Database API makes versus the Entity API!

Just helping out with triage where possible. This seems to be definitively fixed by #20/21, no?

I feel there are actually a couple different issues with i18n telephone validation. The fixes should happen within the webform_clientside_validation submodule, no?

Hey Liam,

In the coming weeks I'll be doing a screen cast about our experience in theming out a somewhat complete feature set of Webform functionality.

There were a few other probable IE-isms that popped out at us (some datetime stuff, specifically). I wonder if a meta might be in order to group that effort?

I believe that the root cause is that Drupal's XSS utility is somewhat dated. There are a pile of "safe" HTML5 tags that are stripped out -- a subset of SVG's elements among them.

This is a somewhat heavy-handed approach that immediately comes to mind to resolve this in the mean-time:

    $status_messages = ['#type' => 'status_messages'];
    
    // Rather than concatenating things in `#prefix`, extract the
    // existing prefix and only run XSS filter on that.
    $prefix = $form['#prefix'] ?? '';
    if (!($prefix instanceof MarkupInterface)) {
      $prefix = Xss::filterAdmin($prefix);
    }
    
    // Then render a build array noting that...
    //  1. $prefix has been sanitized.
    //  2. $status_messages has been sanitized (via Twig)
    $build = [
      ['#markup' => Markup::create($prefix)],
      $status_messages,
      array_filter($form, static fn($key) => $key !== '#prefix', ARRAY_FILTER_USE_KEY),
    ];
    $output = $renderer->renderRoot($build);

Luke.Leber → created an issue.

Additionally manually confirmed attribute coverage against a "real world" site (via Webform).

• Audio files
• Document files
• Generic files (this is the core render element!)
• Image files
• Video files

Cheers

Fingers crossed, test bot.

Approach-wise, I feel that this logic belongs in \Drupal\file\Element\ManagedFile::processManagedFile class versus the FormBuilder because the file module provides the managed_file element plugin.

This change needs tests. It would also be great to test Webform to ensure that the elements that extend managed file are also swept up and fixed.

Luke.Leber → changed the visibility of the branch 2998857-additional-information-needed to hidden.

Luke.Leber → made their first commit to this issue’s fork.

saschaeggi → credited Luke.Leber → .

#68 seems to outline another instance where treating all oembed providers equally simply doesn't work in practice.

In my mind this adds a lot more weight to the issues floating around that support altering various inputs and outputs of the oembed media system.

joegl, feel free to open an issue with https://www.drupal.org/project/oembed_lazyload → and we may be able to make this accessibility enhancement available with no code and/or patches for end-users faster than core is able to adapt to the need. 👍

I, the module maintainer, would like to opt out of automated suggestions for this module. It's highly unlikely that anything that the bot produces will actually work here.

See https://www.drupal.org/project/diff_plus/issues/3447124 📌 [PP-1] Drop support for Drupal 9; Add support for Drupal 11 Postponed to track compatibility.

Setting postponed on https://www.drupal.org/project/diff/issues/3429862 📌 Automated Drupal 11 compatibility fixes for diff Needs review

Luke.Leber → created an issue.

Thanks for the bug report. Unfortunately, I wasn't able to reproduce a White Screen of Death as in #2962941, but I was able to trigger an E_WARNING by manually editing the view mode to a nonsensical value.

Warning: Undefined array key "this-view-mode-does-not-exist" in Drupal\diff_plus\Plugin\diff\Layout\VisualInlineHtml5DiffLayout->getDisplayModeOptions() (line 154 of modules/contrib/diff_plus/src/Plugin/diff/Layout/VisualInlineHtml5DiffLayout.php).
Drupal\diff_plus\Plugin\diff\Layout\VisualInlineHtml5DiffLayout->getDisplayModeOptions(Object, Object, Object, 'this-view-mode-does-not-exist') (Line: 234)
Drupal\diff_plus\Plugin\diff\Layout\VisualInlineHtml5DiffLayout->build(Object, Object, Object) (Line: 168)
Drupal\diff\Controller\PluginRevisionController->compareEntityRevisions(Object, Object, Object, 'visual_inline_html5') (Line: 49)
Drupal\diff\Controller\NodeRevisionController->compareNodeRevisions(Object, Object, Object, 'visual_inline_html5')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 53)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Is this what you were seeing?

In order to fix a bug, we'll first need a failing test. Can you update the steps to reproduce so that I can write such a test case?

Thanks!

Looking at the upstream issue, there was a valid point made that <h2><div>content</div></h2> isn't legal html.

That may help explain why this hasn't been addressed yet. Given how long this has been open, might it be an option for folks suffering from this to consider migrating to markup like <h2><span>content</span></h2> instead? I'm not convinced that cksource will make divs in headings work as a product feature.

!48 looks good to me. I understand this is basically impossible to test, but there shouldn't be any harm in delaying the collection of settings.

We've sporadically experienced weird "should never happen" things on the platform, so after this lands, I'll keep this issue in mind and chime in if another inexplicable thing (i.e. cache corruption) happens afterwards. I think that's the only reasonable evaluation we can do against the theory written out in the I.S.

A b/c break was inadvertently added by https://git.drupalcode.org/project/drupal/-/commit/6a43b511df9e26aa5abf0... when jquery was swapped out for vanilla JS.

Previously, jQuery was more tolerant of an element not found condition. The vanilla JS results in an uncaught exception.

I was going to remove the needs tests tag, but Dave beat me to it :D

Ahh, I see - the legacy option can still use the new placeholder strategy effectively. I'll get a patch whipped up when I have a free moment.

Hey Eduardo,

I see you're still using the legacy formatter option. To preserve backwards compatibility until I can roll a 3.x branch, I added some additional field formatter settings that should be used to leverage the new thumbnails. Can you try any of the other configurations out to see if they work for your theme?

I know it can be a little confusing; I'm trying to balance not breaking backwards compatibility for existing users with providing intuitive defaults for new users.

Luke.Leber → created an issue.

Fairly safe to assume this one's outdated, as Google Optimize isn't a product anymore :-).

Thanks, @greggles.

Which page?

https://developers.google.com/tag-platform/devguides/gtag-integration

For the record, I wholeheartedly believe that the use of the developer_id property is innocuous here on the part of the module maintainers. I was simply seeking more information on it.

The Google documentation uses language like "your customers", which implies somewhat of a different relationship with end-users than is traditionally implied by FOSS Drupal modules that have fluid maintainers.

Updated I.S. with answers (best that can be done with known information).

There may be some level of difference being that the Magento developer ID is likely under the umbrella of the Adobe Corporation whereas the google_tag module module doesn't really have a well known "legal entity" -- for lack of a better term -- behind it.

I appreciate anything you can dig up on this. Thanks

It's something new that's never been seen before on our team and there is precious little information available on this other than one page from Google explaining how, but nothing beyond that.

The developer ID request form is seemingly bound to a Google account of an individual.

Matt, can you link to any google.com hosted documentation that confirms what is said in #5 about access policies?

Luke.Leber → created an issue.

Potentially related issue on the same note as #22, albeit not in Middleware, but a different service constructor.

NW for test fails.