🇬🇧United Kingdom @altcom_neil

Fix typo in title

altcom_neil → created an issue.

Hi, the simplest fix I found was simply to check the value is not empty and set the attribute to NULL if it is:

diff --git a/block_title_link.module b/block_title_link.module
index c56cfba..351ec7e 100644
--- a/block_title_link.module
+++ b/block_title_link.module
@@ -93,8 +93,8 @@ function block_title_link_preprocess_block(&$variables) {
         '#title' => $blockLabel,
         '#url' => Url::fromUri($tileLink),
         '#attributes' => [
-          'target' => $linkTarget,
-          'title' => $linkTitle,
+          'target' => $linkTarget ?: NULL,
+          'title' => $linkTitle ?: NULL,
         ],
       ];
     }

The attributes are then not rendered if the value is NULL.

altcom_neil → created an issue.

MediaLibraryUiBuilderInterface doesn't exist yet - https://www.drupal.org/project/drupal/issues/3359354 📌 Add an interface to media_library.ui_builder so the service can be decorated properly Needs work

Hi,

Drupal 10.3.5 | Editor Advanced Link 2.2.6 | CKEditor 5

Is this issue now fixed in the contrib module? We were using patch #9 which was causing js errors now. I have tried both MR14 and MR17 and both have the issues.

MR14 - correctly lays everything out but has console errors.

MR17 - leaves the 'Open in a new window' option outside of the Advanced tab.

I removed the patches and the functionality appears to work without any issues.

Hi,

Just to confirm this patch is causing a fatal error in D10.3.5 / PHP8.2+ due to trying to look for an array key in a NULL.

I would suggest that the whole function is replaced with one that uses hook_form_FORM_ID_alter as there is no reason for it to run for any form but the one that is being checked for. Then it checks for the array existing before trying to check for the key.

diff --git a/computed_field_plugin.module b/computed_field_plugin.module
index 5a86592..8b14ee4 100644
--- a/computed_field_plugin.module
+++ b/computed_field_plugin.module
@@ -47,39 +47,38 @@ function computed_field_plugin_entity_bundle_field_info(EntityTypeInterface $ent
 }
 
 /**
- * Implements hook_form_alter().
+ * Implements hook_form_FORM_ID_alter() for field_ui_field_storage_add_form.
  */
-function computed_field_plugin_form_alter(&$form, FormStateInterface $form_state, $form_id) {
-  /**
+function computed_field_plugin_form_field_ui_field_storage_add_form_alter(&$form, FormStateInterface $form_state, $form_id) {
+  /*
    * Remove Computed render array field type from available options.
    *
    * @todo: Remove this alteration when related core issue is fixed.
    * @link: https://www.drupal.org/project/drupal/issues/2932273
    */
-  if ($form_id === 'field_ui_field_storage_add_form') {
-    if (version_compare(\Drupal::VERSION, '10.1.999', '<')) {
-      // Remove Computed render array field type from "Add a new field".
-      foreach ($form['add']['new_storage_type']['#options'] as $category => $fields) {
-        foreach ($fields as $key => $value) {
-          if ($key === 'computed_render_array') {
-            unset($form['add']['new_storage_type']['#options'][$category][$key]);
-          }
+  if (version_compare(\Drupal::VERSION, '10.1.999', '<')) {
+    // Remove Computed render array field type from "Add a new field".
+    foreach ($form['add']['new_storage_type']['#options'] as $category => $fields) {
+      foreach ($fields as $key => $value) {
+        if ($key === 'computed_render_array') {
+          unset($form['add']['new_storage_type']['#options'][$category][$key]);
         }
       }
+    }
 
-      // Remove Computed render array field type from "Re-use an existing field".
-      if (isset($form['add']['existing_storage_name']) && isset($form['add']['existing_storage_name']['#options'])) {
-        foreach ($form['add']['existing_storage_name']['#options'] as $key => $value) {
-          $arguments = $value->getArguments();
-          if ($arguments['@type']->getUntranslatedString() === 'Computed render array') {
-            unset($form['add']['existing_storage_name']['#options'][$key]);
-          }
+    // Remove Computed render array field type from "Re-use an existing field".
+    if (isset($form['add']['existing_storage_name']) && isset($form['add']['existing_storage_name']['#options'])) {
+      foreach ($form['add']['existing_storage_name']['#options'] as $key => $value) {
+        $arguments = $value->getArguments();
+        if ($arguments['@type']->getUntranslatedString() === 'Computed render array') {
+          unset($form['add']['existing_storage_name']['#options'][$key]);
         }
       }
     }
-    elseif (array_key_exists('computed_render_array', $form['add']['new_storage_type'])) {
-      // Remove Computed render array field type from "Add a new field".
-      unset($form['add']['new_storage_type']['computed_render_array']);
-    }
+  }
+  elseif (isset($form['add']['new_storage_type']) && is_array($form['add']['new_storage_type'])
+    && array_key_exists('computed_render_array', $form['add']['new_storage_type'])) {
+    // Remove Computed render array field type from "Add a new field".
+    unset($form['add']['new_storage_type']['computed_render_array']);
   }
 }

Attached is the patch for 8.x-1.4.

Cheers, Neil

Hi,

Thanks for everyone's work on this so far.

Just creating a fixed diff file from the last commit 2643761b so we can test this in multiple development environments (Drupal 10.3.5 / PHP 8.2.18).

We are using this patch now that the https://www.drupal.org/project/readonlymode/issues/3224538 🐛 Config schema errors Fixed has been applied.

Works as intended.

This should be a high priority as this causes a fatal error in PHP 8+.

Sorry, correct patch file for 10.3+/11.x

Hi, thanks for the work so far on this.

We have multiple sites built on the same systems but with some custom user roles. Comparing changes to views between sites has always thrown up the differences in these sections even though none have actually made any changes at all.

The latest patch didn't apply for me (D10.3.2), so I have created just a patch for the \Drupal\views\Plugin\views\filter\FilterPluginBase::validateExposeForm method from the latest D11 code.

This works stripping all of the redundant roles from the config.

Hi, attached is an update of patch #30 for Drupal 10.3.1.

@gcb That's much simpler - updated to that and it is working on Drupal 10.3.1 and Salesforce 5.1.0

We are using patch #23 successfully in production sites.

@Imlima If you were getting the TypeError: implode(): Argument #2 ($array) must be of type ?array, string given in implode() and TypeError: Argument 2 passed to _readonlymode_form_list_check() must be of the type string, array given errors after applying patch #23 then you hadn't run drush updatedb to run readonlymode_update_8002() which converts the 'forms.additional.edit' and 'forms.additional.view' config items into arrays? Or had they errored and failed somehow?

There are now two ways of fixing the issue in patches, up to and including patch #23 and in the merge request it was all about making the two config vars arrays as the code was expecting, patch #25 and #27 are simpler by just changing what the code to expecting the strings as they are in the current config.

Personally I favour the first solution to make the config arrays as that is what the code uses in _readonlymode_form_list_check(). If the config is saved as an array then it is only converted from string to array and back again once in the admin form. When it is being used during normal site operation the config is an array.
By keeping it as a string in the config every time that _readonlymode_form_list_check() is called the string has to be split into an array at that point. Not a massive overhead but it seems that it is better to store the config how it is intended to be used, not just as a consequence of how it is edited in the admin form?

Hi, patch seems to work for us in Drupal 10.3

Attached is a patch file for current changes at comment #30 for ease of use.

Cheers, Neil

Hi,

I don't think what your initial patch does what the update hook was meant to be doing.

I think it was intended to convert the 7.x version of the modules config to the 8.x version. It should be something like:

function readonlymode_update_8001() {
  $settings = \Drupal::configFactory()->getEditable("readonlymode.settings");
  if (isset($settings['site_readonly'])) {
    // Config from the 7.x branch needs updating to 8.x structure.
    $new_settings = [
      'enabled' => $settings['site_readonly'],
      'url' => $settings['site_readonly_url'],
      'messages.default' => $settings['site_readonly_message'],
      'forms.additional.edit' => $settings['site_readonly_forms_allowed'],
      'forms.additional.view' => $settings['site_readonly_forms_viewonly'],
    ];
    $settings->setData($new_settings);
    $settings->save();
    return 'Updated readonlymode.settings configuration to 8.x version.';
  }
  return NULL;
}

I haven't tested this as we didn't use the 7.x version.

In \Drupal\mailgun_webhooks\Controller\MailgunWebhookController the old RequestStack use needs to be replaced with the symfony version.

- use Drupal\Core\Http\RequestStack;
use Drupal\Component\Serialization\Json;
use Drupal\Core\Controller\ControllerBase;
use Symfony\Component\HttpFoundation\Response;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\mailgun_webhooks\Event\MailgunWebhookEvent;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+ use Symfony\Component\HttpFoundation\RequestStack;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;

@Panchuk
Good work on this.

For number 2 raised above you can use a dynamic path. So change the routing to:

# Endpoint for the webhooks.
mailgun_webhooks.endpoint:
  path: '/api/mailgun/{webhook_key}'
  defaults:
    _title: 'Mailgun Webhook'
    _controller: '\Drupal\mailgun_webhooks\Controller\MailgunWebhookController::processWebhook'
  methods: [POST]
  requirements:
    # Allow the endpoint to be accessed without authentication.
    # This is required for Mailgun to be able to access the endpoint.
    _access: 'TRUE'
    _format: 'json'

And then the \Drupal\mailgun_webhooks\Controller\MailgunWebhookController::processWebhook is changed to expect a $webhook_key variable and compare it with a config var so it can be set to a unique value in every site.

  public function processWebhook($webhook_key): Response {
    // If the webhook_key value provided in the URL does not match the one in
    // the current config throw a 401: Unauthorized error.
    if ($webhook_key !== $this->config->get('webhook_key')) {
      $response = new Response('Unauthorized: Webhook key does not match.');
      $response->setStatusCode('401');
      return $response;
    }

I added a hook_install to generate the URL when the module is installed:

function mailgun_webhooks_install() {
  // On install generate a random string to use in the callback URL.
  $random = new Random();
  $webhook_key = $random->name(48, TRUE);
  $config = \Drupal::configFactory()->getEditable('mailgun_webhooks.settings');
  $config->set('webhook_key', $webhook_key);
  $config->save();
}

If someone tries to use the wrong webhook_key you get the 401 error from MailgunWebhookController and if you are missing the webhook_key entirely (using path /api/mailgun) then you just get the standard Drupal 404 page.

Cheers, Neil

Argh - missed a use statement!

Add a version that has a dependency on the patch from MailgunHandlerInterface::sendMail() should throw exceptions on error ✨ MailgunHandlerInterface::sendMail() should throw exceptions on error Active so that the exceptions are also thrown as well as events dispatched.

The improved error handling added in https://www.drupal.org/project/mailgun/issues/3346666 ✨ MailgunHandlerInterface::sendMail() should throw exceptions on error Active only does so much as the exception provides no way of linking back to where the email originating.

Adding a way to track webhooks as here https://www.drupal.org/project/mailgun/issues/3175875 ✨ Trigger Event upon receiving Mailgun Webhook Needs review is only so much use if there is no way to link what was sending the email whether it was a successfully delivered or not. By adding this MailSuccessEvent the id from the API can be used to link the email source with the result of the webhook.

Altcom_Neil → created an issue.

Altcom_Neil → created an issue.

Altcom_Neil → created an issue.

Hi, I have created a new patch based on the latest code in the merge request as I couldn't get the merge request patch to apply.

While we aren't actually using the Gmail Oauth I was trying to add support for using the Key module to provide a more secure way to store the SMTP username and password see https://www.drupal.org/project/smtp/issues/3134249 → .

This module allows me to add a simple submodule that just provides another SmtpProvider so while I am not testing the Gmail Oauth side I am testing the SmtpProvider system and it works like a charm!

One suggestion I would make is to not make the Google APIClient a requirement in the composer file but a hook_requirement() in the smtp_gmail_api submodule:

diff --git a/composer.json b/composer.json
index 030217e..4685a9b 100644
--- a/composer.json
+++ b/composer.json
@@ -8,7 +8,6 @@
     "issues": "https://www.drupal.org/project/issues/smtp"
   },
   "require": {
-    "google/apiclient": "^2.14",
     "phpmailer/phpmailer": "^6.1.7"
   },
   "extra": {
diff --git a/modules/smtp_gmail_api/smtp_gmail_api.install b/modules/smtp_gmail_api/smtp_gmail_api.install
new file mode 100644
index 0000000..4b7de79
--- /dev/null
+++ b/modules/smtp_gmail_api/smtp_gmail_api.install
@@ -0,0 +1,17 @@
+<?php
+/**
+ * Implements hook_requirements().
+ */
+function smtp_gmail_api_requirements($phase) {
+  $requirements = [];
+  if ($phase == 'install') {
+    if (!class_exists('\Google\Client')) {
+      $requirements['google_apiclient_library'] = [
+        'description' => t('Google Gmail API SMTP provider requires the "google/apiclient" library. Please install first from https://github.com/googleapis/google-api-php-client'),
+        'severity' => REQUIREMENT_ERROR,
+      ];
+    }
+  }
+
+  return $requirements;
+}

so that projects not using the submodule don't get the library installed regardless.

I have been looking at how https://www.drupal.org/project/smtp/issues/3102055 ✨ Implement OAuth to support GMail/GSuite accounts Needs review have added support for a Gmail Oauth key authentication and think that utilising that code is a better way to go as it is also being actively developed. I have done a patch that is dependent on that code. If it gets merged in it makes it straight forward to add support for Key based authentication (and any other future authentication).

I have redone the patch for the latest version (8.x-1.2) as patch #24 wouldn't apply.
I have tweaked the code to make the use of the Key module optional.

Updated patch for 2.0.0-beta1 which incorporates issue https://www.drupal.org/project/cloudflare/issues/3401335 🐛 Excessive Tag Hash Collisions RTBC

Sorry, bit more testing and I realised the patch wasn't handling saving an entity correctly when the password input was empty.

Here is a new version of the patch with the code cribbed from \Drupal\Core\Field\Plugin\Field\FieldType\PasswordItem.

I tried including the pre_hashed and existing properties of the PasswordItem field - but could not work out how they were meant to be set and used.

Altcom_Neil → created an issue.

Updated the patch for v5.0.4

@AaronBauman The patch only seems to add the "Salesforce" local task to the Webform Submission admin pages as far as I can tell. Not sure if there are other pages it should be being added to where it either should or should not be.

Hi, we have had exactly the same problem with the closed message having the wrong message type 'status' instead of 'type'.

Attached is patch that adds a config "Message Type overrides" in the Webforms: Advanced configuration > User interface settings.

    $form['ui']['message_types'] = [
      '#type' => 'webform_codemirror',
      '#mode' => 'yaml',
      '#title' => $this->t('Message Type overrides'),
      '#description' => $this->t('Override the message types from their default values in code. Use Yaml, one per line e.g. <code>form_close_message: warning

'),
'#default_value' => $config->get('ui.message_types'),
];

In Drupal\webform\WebformMessageManager::apply Drupal\webform\WebformMessageManager::display the config is then retrieved and allows you to replace the hardcoded message type:

      $webform_message_types = Yaml::decode($this->configFactory->get('webform.settings')->get('ui.message_types') ?: '') ?: [];
      if (!empty($webform_message_types[$key])) {
        $type = $webform_message_types[$key];
      }

Just an update from the latest patch the removes the use of the dynamic assigned class property to stop deprecation notices in PHP 8.2 https://wiki.php.net/rfc/deprecate_dynamic_properties. In PHP 9 this will cause fatal errors.

Hi

We also ran into this issue and another related issue - cache tags in the same Cloudflare account will clear all environments - so the UAT sites cache tags will clear the production sites cache if you are using the same account during development. We have added a patch that allows you to prefix the cache tag with an environment character so that cache tags are unique per environment.
See https://www.drupal.org/project/cloudflare/issues/3394651 ✨ Add optional Environment setting Needs review

In that code we increased the length of the hashed cache tag (before adding the environment character) to 6 characters (giving 16.7 million unique codes) - as we didn't spot the better improvement of using the larger character set that you have used here. We have been using this code on sites with in the excess of 100,000 nodes and we haven't run into any header size issues so 4 character tags should be fine.
If you do use 6 characters in the hash then you are up to over 2 billion unique hashes!

Should the length of the hash value be a config value - with a minimum of 4 so that it can be configured on a site-by-site basis? Very, very, very, very (etc) large sites would potentially have more than 1.6 million cache tags if they have millions of entities?

Cheers, Neil

New version of this patch using the cache tag hash code from https://www.drupal.org/project/cloudflare/issues/3401335 🐛 Excessive Tag Hash Collisions RTBC to use full alphanumeric charset so that you get more unique hashes - over 2.17 billion!

Hi all,

Sorry if this is cross pollinating with another issue but we had exactly this issue after migrating to D9 and updating to the new version of the Redirects module. It caused a lot of confusion with clients trying to manage their content as in D7 the alias redirects worked but not in D9 after the upgrade.

One way we solved this was to integrate not only showing the redirects to a node but the redirect away as well. This makes it easier for a user when editing the node to see it is being redirected to another URL. And this simplifies the creation of a redirect from an old node to a new one.

There is a patch here that some people may find useful https://www.drupal.org/project/redirect/issues/3386107 ✨ Simplify how to redirect from one node to another node or URL Active

Updated version of the patch for 5.0.3

Still not sure why there are two vars - one configurable per mapping \Drupal\salesforce_mapping\Entity\SalesforceMapping::$push_retries and one that is set during install and as far as I can tell not configurable \Drupal\salesforce_push\PushQueue::$maxFails?

Altcom_Neil → created an issue.

@catch One thing with the commit https://git.drupalcode.org/project/drupal/-/merge_requests/4853/diffs?co...

It looks like the $ has gone awol in $ajax_page_state['theme_token']:

    if ($theme === $this->configFactory->get('system.theme')->get('default') || $this->csrfGenerator->validate(ajax_page_state['theme_token'], $theme)) {

@commit That commit for Drupal\Core\Theme\AjaxBasePageNegotiator::determineActiveTheme is a better solution.

For the Drupal\Core\StackMiddleware\AjaxPageState how about something like this?

  /**
   * {@inheritdoc}
   */
  public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE): Response {
    if ($request->request->has('ajax_page_state')) {
      $request->request->set('ajax_page_state', $this->parseAjaxPageState($request->request->get('ajax_page_state')));
    }
    elseif ($request->query->has('ajax_page_state')) {
      $ajax_page_state = $this->parseAjaxPageState($request->query->get('ajax_page_state'));
      $request->query->set('ajax_page_state', $ajax_page_state);
      $request->request->set('ajax_page_state', $ajax_page_state);
    }
    return $this->httpKernel->handle($request, $type, $catch);
  }

  /**
   * Parse the ajax_page_State variable in the request.
   *
   * Decompresses and decodes the json to return an array.
   *
   * @param mixed $ajax_page_state
   * This should be a string containing the compressed json query parameters.
   *
   * @return array
   */
  private function parseAjaxPageState(mixed $ajax_page_state) {

    if (is_string($ajax_page_state)) {
      $ajax_page_state = json_decode(UrlHelper::uncompressQueryParameter($ajax_page_state), TRUE);
    }
    else {
      /**
       * @deprecated ajax_page_state should be a compressed json encoded string
       * in all POST and GET in Drupal 10.2.0 and so the passed in variable will
       * be a string only in 11.x
       */
      trigger_error('From 10.2.0 Drupal\Core\StackMiddleware\AjaxPageState::handle() expects the ajax_page_state to be compressed using UrlHelper::compressQueryParameter() in all post and get requests, this check will be removed in drupal:11.x and generate a TypeError instead. See https://www.drupal.org/node/3389367.', E_USER_DEPRECATED);
    }
    return $ajax_page_state;
  }

In Drupal 10.2.x the code would allow the uncompressed ajax_page_state to pass through while logging the deprecated error. In Drupal 11.x the method could be change to private function parseAjaxPageState(string $ajax_page_state) and then it would throw a type error if the ajax_page_state variable is not the expected compressed string? Or 11.x could remain as it is now and just throw the TypeError from UrlHelper::uncompressQueryParameter()?

The new method parseAjaxPageState() to does all of the checking, then decompressing, then decoding to return the expected array to remove any duplicated code in the main handle() method and allows the adding of a single call to trigger_error()?

New version of the D9.5.x with your latest change to AjaxBasePageNegotiator::determineActiveTheme plus the parseAjaxPageState method to handle code not compressing the ajax_page_state variable attached.

@catch Re the change in #66 to the Drupal\Core\Theme\AjaxBasePageNegotiator::applies, if the && isset($ajax_page_state['theme_token']) is removed then we have to do something about line 78 in Drupal\Core\Theme\AjaxBasePageNegotiator::determineActiveTheme because every call to this will cause a PHP warning.

"Warning: Undefined array key "theme_token" in Drupal\Core\Theme\AjaxBasePageNegotiator->determineActiveTheme() (line 78 of .../core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php)

It could just be a simple as changing it to

   public function determineActiveTheme(RouteMatchInterface $route_match) {
     $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
     $theme = $ajax_page_state['theme'];
-    $token = $ajax_page_state['theme_token'];
+    $token = $ajax_page_state['theme_token'] ?? '';
 
     // Prevent a request forgery from giving a person access to a theme they
     // shouldn't be otherwise allowed to see. However, since everyone is

as $this->csrfGenerator->validate() expects the $token var to be a string? So if the theme is not the default AND the theme_token is not set the var is an empty string.

We were also getting errors

TypeError: Drupal\Component\Utility\UrlHelper::uncompressQueryParameter(): Argument #1 ($compressed) must be of type string, array given, called in .../core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php on line 35

due to the ajax_page_state already being uncompressed. This was coming from an old custom ajax callback that did ajax get requests before issue #956186 📌 Allow AJAX to use GET requests Fixed that we have now changed but just for defensiveness how about a condition before the code tries to uncompress the var:

   public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE): Response {
     if ($request->request->has('ajax_page_state')) {
       $ajax_page_state = $request->request->get('ajax_page_state');
-      $ajax_page_state = json_decode(UrlHelper::uncompressQueryParameter($ajax_page_state), TRUE);
+      if (is_string($ajax_page_state)) {
+        $ajax_page_state = json_decode(UrlHelper::uncompressQueryParameter($ajax_page_state), TRUE);
+      }
       $request->request->set('ajax_page_state', $ajax_page_state);
     }
     elseif ($request->query->has('ajax_page_state')) {
       $ajax_page_state = $request->query->get('ajax_page_state');
-      $ajax_page_state = json_decode(UrlHelper::uncompressQueryParameter($ajax_page_state), TRUE);
+      if (is_string($ajax_page_state)) {
+        $ajax_page_state = json_decode(UrlHelper::uncompressQueryParameter($ajax_page_state), TRUE);
+      }
       $request->query->set('ajax_page_state', $ajax_page_state);
       $request->request->set('ajax_page_state', $ajax_page_state);
     }
-- 

not sure if it is necessary but there might be other ajax code floating around in contrib modules doing a similar thing to what we used to do?

Attached is another 9.5.x backport with the latest commits to MediaLibraryState and AjaxBasePageNegotiator, plus the above defensive bits of code to stop any potential PHP warnings being thrown. D9.5.x patch is now again dependant on the patch here https://www.drupal.org/project/drupal/issues/956186#comment-14991435 📌 Allow AJAX to use GET requests Fixed

Thanks for working on this, here is a D9.5.x backport of patch #11

@catch
I have pulled in the latest changes you have committed to your 11.x issue branch except the changes in https://git.drupalcode.org/project/drupal/-/merge_requests/4853/diffs?co... as the comment says '@todo Uncomment new method parameters before drupal:11.0.0.' so I have left them as they were.

Re your restoring of the isset - not sure if you intended to write it that way or not but the change at https://git.drupalcode.org/issue/drupal-3348789/-/commit/a1c7a99b0d2eb2c... is wrapping the isset() within the !empty():
return !empty($ajax_page_state['theme'] && isset($ajax_page_state['theme_token']))
Original code was:
return !empty($ajax_page_state['theme']) && isset($ajax_page_state['theme_token']);

Removing the code change from AjaxBasePageNegotiator has the happy by product of removing the dependency on Allow AJAX to use GET requests [#956186] 📌 Allow AJAX to use GET requests Fixed as the patch no longer trying to change the line under the one this issue patches in D9.5.x

Also - confirming that the latest changes fix the issue reported in #50 with the uncompressed ajax_page_state values being passed in the Media Library view. On D9.5.x I was getting an error trying to use the bulk operation form but that is now fixed.

Attached are new Drupal 9.5.x versions without the change to the condition in AjaxBasePageNegotiator::applies

@catch Just going back to #36
I included that change in the 9.5.x patch but I'm not sure that change removing the isset($ajax_page_state['theme_token']) should be there.

The reason that the 'theme_token' var is missing in the ajax requests is because the System module deliberately excludes it when building the 'ajaxPageState' settings in system_js_settings_alter():

      // The theme token is only validated when the theme requested is not the
      // default, so don't generate it unless necessary.
      // @see \Drupal\Core\Theme\AjaxBasePageNegotiator::determineActiveTheme()
      $active_theme_key = \Drupal::theme()->getActiveTheme()->getName();
      if ($active_theme_key !== \Drupal::service('theme_handler')->getDefault()) {
        $settings['ajaxPageState']['theme_token'] = \Drupal::csrfToken()
          ->get($active_theme_key);
      }

So for the default theme the theme token is not added, and so

  public function applies(RouteMatchInterface $route_match) {
    $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
    return !empty($ajax_page_state['theme']) && isset($ajax_page_state['theme_token']);
  }

returns false for the default theme which I guess is to 1. remove some unnecessary data from the ajax object; and 2. skip calling the determineActiveTheme method which is just going to return the default theme. This means that ThemeNegotiator::determineActiveTheme fallsback all the way to the DefaultNegotiator::determineActiveTheme which just returns the default theme.
When we remove the theme_token check from the conditional in AjaxBasePageNegotiator::applies it leads to a php notice as the AjaxBasePageNegotiator::determineActiveTheme expects it to be set when it does $token = $ajax_page_state['theme_token'];.

Worked out that my backport patch was simply missing the new core/lib/Drupal/Core/StackMiddleWare/AjaxPageState.php file and service config, sorry.

I have also added a tweak to the core/lib/Drupal/Core/EventSubscriber/AjaxResponseSubscriber.php that is already part of D10 core to use $event->getRequest()->get(static::AJAX_REQUEST_PARAMETER).

Attached are the new versions of the D9.5.x patches.

I must have missed something that is required for D9.5.x that isn't needed in the D10 & D11 patches as trying to use it causes js errors in AjaxResponse due to all of the libraries being included, which throws the "LoadJS" in core/assets/vendor/loadjs/loadjs.min.js, this stops the other js files from loading that did actually need loading.

I guess I have found why this hasn't been done before. It is not the simple case of adding the support to initiate the callbacks to use GET instead of POST by mimicking what was done for links.

While it looks like all the JavaScript libraries are being added to the AjaxResponse object, the add_js: function add_js(ajax, response, status) call in core/mis/ajax.js throws the "LoadJS" error from the core/assets/vendor/loadjs/loadjs.min.js file when attempting to load any libraries that weren't already loaded.

In our case we were creating a form with an ajax field with a url:

    // Origin
    $form['origin'] = [
      '#type' => 'select',
      '#title' => $this->t('Origin'),
      '#options' => ['' => '- Choose -'] + $origins
      '#required' => true,
      '#attributes' => [
        'class' => [
          'no-error',
        ],
        'data-ajax-type' => 'get',
      ],
      '#ajax' => [
        'disable-refocus' => TRUE, // Or false to re-focus on the triggering element.
        'event' => 'change',
        'progress' => [
          'type' => 'throbber',
          'message' => $this->t('Updating ...'),
        ],
        'url' => Url::fromRoute('custom_module.origin_callback.get'),
        'options' => ['query' => ['ajax_form' => 1]],
      ],
    ];

The Controller for this route then updated a second select field with a list destinations with a ReplaceCommand, and returned a MessageCommand. It is the 'core/drupal.message' library that is in the list of attached libraries with the AjaxResponse but throws the error trying to load it. Which then leads to a further js error when the message call message: function message(ajax, response) tries to call 'new Drupal.Message()'.

This could be a trivial or a complicated fix requiring more patches, I don't have any familiarity with any of the Ajax system, nor the time to investigate any further so will have to give up on adding this functionality for now.

Re #41
Yes because I am already adding the patch https://www.drupal.org/files/issues/2023-03-30/956186-174-reroll-95-185.... → from 📌 Allow AJAX to use GET requests Fixed the patch doesn't work so attached is an additional version that is dependent on that patch from 📌 Allow AJAX to use GET requests Fixed .

Thanks for this work on this so far everyone. If anyone else needs it for D9 I have created the patch based on 3348789-37.patch. We have been looking at the various patches for D9 to allow Ajax to make get requests so need this to wrangle the query parameters into a nicer looking (and shorter) string.

Altcom_Neil → created an issue.

Altcom_Neil → created an issue.

Altcom_Neil → created an issue.

Altcom_Neil → created an issue.

I have tested the patch in #53 with Drupal 9.5.9 and it works, being as it is the same solution as provided for Entity Embeds drupal-entity tags it would seem to be the solution to go for.
Though we are still using the ckeditor bundled in the core not the contrib so I had to manually apply the changes to the core version js.

Attached is the version of the patch for the core but using the same fix as #53.

altcom_nh → created an issue.

altcom_nh → created an issue.

altcom_nh → created an issue.