jQuery Cookie 1.4.1 replace with Js-Cookie 3.0.5 and Shim

The last submitted patch, 2: js-cookie-shim-3382000.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

With updated tests as descriptions were changed on Cookie path

Interesting suggestion, thanks.

I think this looks good.

I'd like to see a couple of tweaks:

'#description' => t('Example: %url or %path or in early testing js.cookie and D9 shim with %jscookie', array(

• Let's drop "early testing"; I'd be more comfortable saying "experimental"?
• Can we provide a link to explain more about the shim - perhaps to: https://www.drupal.org/node/3104677 →
• I am a bit nervous about this:

    if (strpos( $custom_path,  'js-cookie')){
      // if js.cookie is being used to replace jquery.cookie 1.4.1, then use this shim from a Drupal 9.5.10 environment
      $libraries['cookie']['js']['misc/jquery.cookie.shim.js']['data'] = $path . '/replace/ui/external/jquery.cookie.shim.js';
    }

(nit - there's an extra space before the variable with the if condition.)

It's probably okay to check for this string in the custom path, but that could easily go wrong e.g. if someone stores a local copy of the library and renames it without that exact pattern.

If we're going to rely on the presence of that string, let's be explicit about it (in the admin UI), or let's add a checkbox instead to indicate that the shim should be included... that might also give us some space in the admin UI to better explain what this is all about.

Finally, it'd be great to see some additional testing of this option being used.

Quick update based on @mcdruid #9. Further explanation of this experimental option, path requirements and link to Drupal 9 Shim. Happy to go deeper with a checkbox in admin interface, if necessary.

Agreed please test! I have only Drupal core based cookies needs, so any custom/module based cookie code would be ideal.

Currently the MR is failing tests, so I think we need to fix that failure first. Thanks!

I need help modifying that test...changed description of field and tried updating, but something else is needed.

The MR still has the "in early testing" wording in the admin UI (see #9) and IMHO putting *** into the admin text for emphasis looks quite messy. Let's try to keep everything as clean and simple as possible.

We also mentioned adding a checkbox for the shim rather than relying on detecting a string in the custom path.

Finally... should the shim still be based on Drupal 9.5? Is it still in D11? If so, can we update the shim and any references, or explain why it's specifically a D9 shim we're including if there's a reason for it to be "locked" etc..

There are a few things to address in the comments, plus needs tests.

I had a bit of time to tidy this up, but I'd still like to see some new tests added to verify that e.g. the checkbox / variable for the shim works as it's supposed to.

So leaving at NW + Needs tests for now.