Account created on 2 March 2016, over 8 years ago
#

Recent comments

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Making a quick update to the existing patch to work with the new function format. While the patch was still applying before, the variable names in the getSourceData function had changed so the code wasn't actually doing anything.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Fixing issue with custom checkbox saving

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I ran into this too. I didn't delete the old one since people could, in theory, still be using it (I think?) but I added a new sub module that works against IP2Location.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Patches 51 and 52 aren't working for me. I'm confused on some of the changes there. Patch 51 mentioned that it covers more bases, but the only change I see was this:

+      elseif (isset($view->element['#viewsreference']['entity'])
+        && $view->element['#viewsreference']['entity'] instanceof EntityInterface) {
+        $replacements[$view->element['#viewsreference']['entity']
+          ->getEntityTypeId()] = $view->element['#viewsreference']['entity'];
+      }

$view->element['#viewsreference']['entity'] is a holdover from an older patch though, it's not being set anywhere in this patch. Then 52 adds an additional check to fix ajax pagination:

-  $viewsreference_plugin_manager = \Drupal::service('plugin.manager.viewsreference.setting');
-  $plugin_definitions = $viewsreference_plugin_manager->getDefinitions();
-  if (isset($view->element['#viewsreference']['enabled_settings'])) {
+  if (isset($view->element['#viewsreference']['entity'])) {
+    $viewsreference_plugin_manager = \Drupal::service('plugin.manager.viewsreference.setting');
+    $plugin_definitions = $viewsreference_plugin_manager->getDefinitions();

Because of that check, and ['entity'] never being set, these plugins never execute for me anymore.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

This was addressed in πŸ“Œ Automated Drupal 10 compatibility fixes Fixed so closing this ticket to avoid confusion.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Re-rolling for latest version

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Yup, I just wanted to fix the issues first. I'll try to get a unit test going, but I have very little experience with it so it might take a bit.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Fixing failing unit test.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Attempting to fix the failures. I assume we'll want to wait until it's confirmed this is the approach people want before we write test cases though.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I also experienced the same error from this patch. I think the patch has a couple different issues. First, it looks like a lot of the derived suggestion logic was pulled from the twig.engine debug code, but the problem with that is that the debug output is presented in reverse order.

    $derived_suggestion = $original_hook;
    $derived_suggestions[] = $derived_suggestion;
    while ($pos = strrpos($derived_suggestion, '__')) {
      $derived_suggestion = substr($derived_suggestion, 0, $pos);
      $derived_suggestions[] = $derived_suggestion;
    }

This would generate hooks in the order of

base_hook__specific__really_specific
base_hook__specific
base_hook

When the later code merged this array with the suggestions array coming from the theme system (which is in the opposite order) it would create odd sorting under some conditions.

    $derived_suggestions_excluding_base_hook = array_slice($derived_suggestions, 0, -1);
    if (isset($info['base hook']) && !in_array($hook, $derived_suggestions_excluding_base_hook)) {
       $suggestions[] = $hook;
     }
    $suggestions = array_merge($derived_suggestions_excluding_base_hook, $suggestions);

Second, the patch had this check:

    $derived_base_hook = array_slice($derived_suggestions, -1);
    if (!in_array($derived_base_hook, $suggestions)) {
      $suggestions = array_merge($derived_base_hook, $suggestions);
    }

The above code would always fire since array_slice returns an array not a scaler value, which could sometimes cause duplicates.

Third, part of the updates removed some variable initialization in twig.engine

-      
-       // Get the value of the base hook (last derived suggestion) and append it
-      // to the end of all theme suggestions.
-      $base_hook = array_pop($derived_suggestions);

Later in the twig.engine file, the base_hook variable is used to determine whether a suggestion is valid or not

      $base_hook = $base_hook ?? $variables['theme_hook_original'];
      foreach ($suggestions as $key => &$suggestion) {
        // Valid suggestions are $base_hook, $base_hook__*, and contain no hyphens.
        if (($suggestion !== $base_hook && !str_starts_with($suggestion, $base_hook . '__')) || str_contains($suggestion, '-')) {
          $invalid_suggestions[] = $suggestion;
          unset($suggestions[$key]);
          continue;
        }
        $template = strtr($suggestion, '_', '-') . $extension;
        $prefix = ($template == $current_template) ? 'x' : '*';
        $suggestion = $prefix . ' ' . $template;
      }

That's the reason folks started getting the "INVALID FILE NAME SUGGESTIONS:" error after applying the patch. I made a new patch that addresses those 3 issues.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I believe the approach for patch #6 is correct here. Looking through the patch in #2838457: Re-enable the default value functionality for Address fields β†’ , it was more than deprecating this functionality. They actually removed all the code that fired the event, so it hasn't been doing anything since version 1.5 of Address.

The original verbiage change to the function header comment made that more clear, before it was updated to the more generic deprecation description in a later issue.

diff --git a/src/Event/InitialValuesEvent.php b/src/Event/InitialValuesEvent.php
index a40c322..842cb6e 100644
--- a/src/Event/InitialValuesEvent.php
+++ b/src/Event/InitialValuesEvent.php
@@ -8,8 +8,8 @@ use Symfony\Component\EventDispatcher\Event;
 /**
  * Defines the initial values event.
  *
- * @see \Drupal\address\Event\AddressEvents
- * @see \Drupal\address\Plugin\Field\FieldWidget\AddressDefaultWidget::getInitialValues()
+ * @deprecated No longer fired since 1.5. Use hook_field_widget_form_alter()
+ *             to change the address #default_value instead.
  */
 class InitialValuesEvent extends Event {

Given that, I think doing a simple check of the event constant's existence is a reasonable fix, as it allows us to support the older versions of the Address module without breaking when version 2 is used. I tested patch 6 on one of my sites that use the Address module, and the address content still cloned without issue, so marking this as RTBC

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

@mradcliffe Thank you! I was wondering why I couldn't select this version to run tests against.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Rerolling for the latest version and taking into account feedback

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

This should be resolved in 2.0.1

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Fixing some incorrect variable names (A couple spots were still using $this->itemSelector instead of $item_selector) and fixing another edge case where the item_selector is blank.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Thanks all! Did some testing on my side as well and it looks good.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Comment 100 introduced a logic change to the getSourceData function in the Json data parser plugin that I believe wasn't intended.

     // Otherwise, we're using xpath-like selectors.
     $selectors = explode('/', trim($item_selector, '/'));
     $return = $this->sourceData;
     foreach ($selectors as $selector) {
+      if (!isset($return[$selector])) {
+        return NULL;
+      }
       if (!empty($selector) || $selector === '0') {
         $return = $return[$selector];
       }

With that change, it's no longer possible to use the Json data parser without setting an item selector. This is because the 'explode' method will return an array with one empty element if an empty item selector is passed to it, which causes the new check to fail 100% of the time and returns null. I've changed that logic to what's below, which I believe solves that problem and keeps the original intended functionality.

    // Otherwise, we're using xpath-like selectors.
    $return = $this->sourceData;
    if (!empty($item_selector)) {
      $selectors = explode('/', trim($item_selector, '/'));
      foreach ($selectors as $selector) {
        if (!isset($return[$selector])) {
          return NULL;
        }
        $return = $return[$selector];
      }
    } elseif (sizeOf($return) === 0) {
      return NULL;
    }
πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Is this patch backwards compatible with 8 and 9, or do we need to make a new Drupal 10 specific module release?

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

This was most likely fixed by #3117872: Form out of sync with more recent Taxonomy module updates β†’ , so closing this out. Feel free to reopen if this is still an issue.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I'm pretty sure this was fixed because the module has changed a lot in the last 6 years, so closing this out.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

The merged D10 patch included these updates, so closing this out.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

It looks like the module owner must have made these updates at some point in the last few years, so closing this out.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Hi Alberto,
I was just checking in to see if you had heard anything back from Jeetendra.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I actually had an issue open for this too, just recently moved over to the production ownership queue due to lack of responses #3371661: Offering to co-maintain Taxonomy Multi-delete Terms β†’ . I don't believe I'll be granted permissions to add other co-maintainers, but assuming the request goes through D9/D10 compatibility will be the first thing I'll be tackling.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I emailed the module owner the same day I opened this issue (3 weeks ago), but I haven't received a response. Switching this to the project ownership queue. Here's a link to the project: https://www.drupal.org/project/taxonomy_multidelete_terms β†’

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

It could be addressed now or in a new issue after this one becomes fixed.

@manuel.adan My vote would be a separate issue. This is holding up a few folks doing upgrades so it would be great to get a new release out and then work on refining afterwards if redirects are still needed.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Just a quick re-roll for the latest version.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Hopefully addressing the last of the permission issues.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Taking into account new permissions in D10.1

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I built on this to hit a few more of the bullet points needed to get it committed. My patch adds the Fixed Blocks tab to the block types admin UI area. It also uses route subscribers and derivatives to alter the new routes for older versions of Drupal, maintaining backward compatibility with older versions. I did alter the routes be under the block type area instead of the block content area for Drupal 10.1. Up the manuel.adan of course but my thoughts were since this is technically configuration and not content it made more sense there. Lastly I updated the tests to hopefully work on all versions of Drupal.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

This has been added in the latest release.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Hi phjou,
I fixed the version requirements and cut a new release, so we should be D10 ready now.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

I had need of this functionality so I put together a patch. Because there's a lot of possible use cases, more then can be handled by a form field, I introduced hooks so that the developer can add support for whatever field they're looking for. One allows for the field to be searched on, the other allows you override the label that gets returned.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

This doesn't appear to be fixed in the latest version. My site is getting the same error and in looking at what's committed into your repo it's not matching the patches people are linking to here.

The current repo looks like this

    $view_arg = !empty($this->configuration['view_arg']) ? $this->configuration['view_arg'] : NULL;
    
    ......

    if (!empty($this->configuration['view_arg'])) {
      $view->setArguments($view_arg);
    }

But this version is missing the part of the other patches where it's wrapping the argument in an array. I believe it should be like this:

    $view->setArguments([$view_arg]);
πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Hi Rick,
That's actually my distro. It's not abandoned or paid, we've just been working on getting it cleaned up for release to the community. One of those things that started as more of an internal use for our agency that grew into something a little more. You must be working with one of our clients, if you need help with it I'm happy to assist, just shoot me a DM on the Drupal slack. It's unclear what version you're on, but the latest version of the distro targets the 2.0 branch of betterlt which fixed the above patch issue.

πŸ‡ΊπŸ‡ΈUnited States jacobbell84

Hi scotwith1t,
That's a good idea on the entity type check. Concerning what was committed though, while different then my original patch, does still address the root cause of the issue as far as I can tell. https://git.drupalcode.org/project/paragraph_view_mode/-/commit/e7ccca92...

Removing the 'string' parameter type was the main part of the fix, which I see there and on the current dev release.

Production build 0.71.5 2024