Atlanta, GA
Account created on 18 April 2008, about 16 years ago
#

Merge Requests

Recent comments

🇺🇸United States paulmckibben Atlanta, GA

I'm also seeing this error when I perform an isEmpty() call in a custom post-update hook.

e.g.

foreach ($nodes as $node) {
    if ($node->field_my_der->isEmpty()) {
      $node->field_my_der->setValue([
        'target_id' => 123,
        'target_type' => 'taxonomy_term',
      ]);
      $node->save();
    }
  }

This code fails with the following errors:

>  [warning] Undefined property: Drupal\dynamic_entity_reference\Plugin\Field\FieldType\DynamicEntityReferenceItem::$entity DynamicEntityReferenceItem.php:495
>  [error]  Drupal\Component\Plugin\Exception\PluginNotFoundException: The "" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of /var/www/html/web/core/lib/Drupal/Core/Entity/EntityTypeManager.php).
🇺🇸United States paulmckibben Atlanta, GA

@richard.thomas Thanks for the patch! I tried it out, and though it seems to keep the redirect from being cached, I now see a different problem with image style derivatives: as an anonymous user, when I load a page that uses image style derivative images, they return 404, and the 404 response gets cached by Drupal's page cache. I see X-Drupal-Cache: HIT among the 404 response headers when the browser requests the image derivative URL.

Checking the file system, the original image has been downloaded, and I can see it in the file system. However, I don't see the image style derivative in the file system. Since Drupal has cached both the page URL and the image derivative URL, subsequent page reloads do not attempt to recreate the derivative image.

Note: if I click "do not cache markup" in the development settings, then the behavior is slightly different: on the first page load, the derivative images are 404. With a normal page refresh, the derivative images load. So part of the problem may be that derivative images are not generated if stage_file_proxy has to retrieve the original file from the origin server. I believe this is still a regression.

This is definitely an improvement, but we still need to figure out how to fix image style derivatives.

🇺🇸United States paulmckibben Atlanta, GA

Adding my request for a new release. Just dealt with a client who lost 2 days of analytics because of this.

🇺🇸United States paulmckibben Atlanta, GA

@sourabhsisodia_ Thank you! Great work. I've merged your MR for this fix.

🇺🇸United States paulmckibben Atlanta, GA

Thanks, @greggles. I did the following to answer your questions.

Test setup in my local ddev environment:

Test #1: using version 2.1.4, does invoking `ddev drush cr` immediately after enabling stage_file_proxy and setting the origin help?
Result: no, it does not help. However, invoking `ddev drush cr` after attempting to load a page with stage_file_proxy enabled, and hard-refreshing the page afterward, does help.

  1. Perform the Test Setup listed above.
  2. Load the homepage in a browser using incognito mode. Note missing images.
  3. Enable stage_file_proxy using drush: `ddev drush en stage_file_proxy`
  4. Set the origin: `ddev drush cset stage_file_proxy.settings origin https://my-origin.example.com`
  5. `ddev drush cr`
  6. Hard-refresh the browser to reload the homepage. The images still do not load.
  7. Hard-refresh the browser again. Still missing the same images.
  8. `ddev drush cr`
  9. Hard-refresh the browser again. Now the images appear.

Test #2: does the problem occur using version 2.1.2?
Result: No.

  1. Disable stage_file_proxy 2.1.4 and remove from codebase
  2. Install in codebase, but do not enable, stage_file_proxy 2.1.2
  3. Perform the test setup listed above.
  4. Load the homepage in a browser in incognito mode. Note missing images.
  5. Enable stage_file_proxy using drush: `ddev drush en stage_file_proxy`
  6. Set the origin: `ddev drush cset stage_file_proxy.settings origin https://my-origin.example.com`
  7. Hard-refresh the browser to reload the homepage. The images load as expected.
🇺🇸United States paulmckibben Atlanta, GA

Thanks, @greggles.

I enable stage_file_proxy manually, when I need it, i.e. `ddev drush en stage_file_proxy`, and then use `ddev drush config-set` to set the origin URL.

I'm curious if that makes a difference. What I see right now is, if an image does not exist in the file system and results in a 404, stage_file_proxy gives us a 302 redirect to the same URL. The 404s repeat, and so do the redirects, until the browser decides it's too many. Those repeated redirects happen pretty quickly, in a split second.

I think it's either the delay in downloading the image from the origin, or a delay in the container's file system, that makes it appear as if the image failed to download, and you still get a broken image.

With "do not cache markup" checked in the development settings, if I reload the page, by that time the image has finished downloading and exists in the container's file system, so it's no longer broken.

I took a look at the code and don't have any good ideas for a fix yet. I'm not sure if introducing a delay is a good idea. And the redirect itself should not be cached if I'm reading the code right. I did see the redirect logic was introduced in 2.1.3, so that lends credence to the correlation between the redirect and this problem.

🇺🇸United States paulmckibben Atlanta, GA

Workaround for this problem is to click "do not cache markup" in development settings.
It will still fail on the first page load, but if you reload the page, the images will load.

🇺🇸United States paulmckibben Atlanta, GA

I'm also experiencing this issue on a DDEV environment. I _think_ the problem may be cache-related. Invoking drush cr results in loading the images successfully.

Browser: Chrome version 124.0.6367.78
OS: MacOS Monterey 12.7.4

Local DDEV:
Drupal core: 10.2.5
PHP: 8.2
stage_file_proxy version: 2.1.4

Errors when attempting to load an image:
Repeating 302 redirects to the same local URL until it stops trying.
Each one shows a cache hit:
X-Drupal-Cache: HIT
X-Frame-Options: SAMEORIGIN

As mentioned above, after drush cr, the problem clears up for the affected images.

🇺🇸United States paulmckibben Atlanta, GA

I fixed the problem with DOMPurify - I updated ColorboxResponsiveFormatter to load DOMPurify if it exists. Thanks to everyone for your work on this patch. It is committed!

🇺🇸United States paulmckibben Atlanta, GA

This is being worked on in Support Responsive Image module in D8 Needs work .

🇺🇸United States paulmckibben Atlanta, GA

Thanks to everyone who has contributed to this patch and gotten it to where it is so far. I just see a couple small problems:

1. With DOMPurify enabled, HTML in captions still get converted to text.
2. colorbox-responsive-formatter.html.twig should use the same aria attributes that colorbox-formatter.html.twig has.

🇺🇸United States paulmckibben Atlanta, GA

Updating issue credits.

🇺🇸United States paulmckibben Atlanta, GA

The goal of this task is completed: GitLab CI can now run tests, and the one failing PHPUnit javascript test has been fixed. I will create follow up issues to fix validation tests (phpcs, eslint, etc.) and to create new, more substantive tests.

🇺🇸United States paulmckibben Atlanta, GA

paulmckibben created an issue.

🇺🇸United States paulmckibben Atlanta, GA

The ckeditor5-stylesheets key was incorrect: it was listed as "ckeditor5_stylesheets" with an underscore, whereas the correct key uses a hyphen.

🇺🇸United States paulmckibben Atlanta, GA

I've merged MR 30. Leaving in RTBC so automated updates can continue.

🇺🇸United States paulmckibben Atlanta, GA

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

🇺🇸United States paulmckibben Atlanta, GA

I have created MR #14 for this issue. Please review and let me know if you have any feedback. Thanks!

🇺🇸United States paulmckibben Atlanta, GA

The issue is present in version 8.x-1.7, which is the latest tagged release.
Even if I apply the patch from Allow to add classes for external and mail links Fixed , the extra CSS classes only get applied if I also apply the icon.
Expected behavior: the extra CSS classes should be applied without requiring the icon to be applied.

🇺🇸United States paulmckibben Atlanta, GA

This was a root cause for an ajax issue I was experiencing, due to two different forms appearing on the same page. On one of the forms, ajax submit failed to get a callback method because the formState->getTriggeringElement() returned NULL.

Once I assigned a unique #name to the submit button for the form that was failing to submit using ajax, the problem was fixed.

Before:

      $form['filter-container']['top-row']['submit'] = [
        '#type' => 'submit',
        '#value' => $this->t('Submit'),
        '#id' => 'license-filter-submit',
        '#ajax' => [
          'callback' => '::ajaxUpdateLicenseResults',
          'wrapper' => 'license-results-container',
          'progress' => [
            'type' => 'fullscreen',
          ],
        ],
      ];

After:

      $form['filter-container']['top-row']['submit'] = [
        '#type' => 'submit',
        '#value' => $this->t('Submit'),
        '#id' => 'license-filter-submit',
        '#name' => 'license-filter-submit',
        '#ajax' => [
          'callback' => '::ajaxUpdateLicenseResults',
          'wrapper' => 'license-results-container',
          'progress' => [
            'type' => 'fullscreen',
          ],
        ],
      ];

Adding a unique '#name' fixed the triggeringElement issue.

Adding this here in hopes that anyone else experiencing this particular use case will find it.

🇺🇸United States paulmckibben Atlanta, GA

This is now committed, and I'll tag a new release 7.x-2.19 shortly.

🇺🇸United States paulmckibben Atlanta, GA

@theturtle Thanks for reporting this. The patch from #29 removed the version information from the library info, which caused the version check to fail. I will add that back and provide a new release shortly.

🇺🇸United States paulmckibben Atlanta, GA

Correct, the library itself is not composer managed. However, there are ways to install the library using composer if you need to. See the comments in 📌 Use composer to install the colorbox and dompurify library Needs work .

🇺🇸United States paulmckibben Atlanta, GA

There's nothing obviously wrong based on your problem description.
I'm not sure why you're trying to invoke the colorbox method as part of attachBehaviors(). Would you want to do that in an event handler instead?

🇺🇸United States paulmckibben Atlanta, GA

@mohit_aghera Thanks for the MR! I agree, the null-safe check is a prudent approach. I have merged your change.

🇺🇸United States paulmckibben Atlanta, GA

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

🇺🇸United States paulmckibben Atlanta, GA

From what I can tell in the Drupal 10 CKEditor media plugin, you need to set the colorbox parameters on the default display, not the media library display. When I did that, everything worked as expected.

🇺🇸United States paulmckibben Atlanta, GA

It looks like success() is a valid method, at least in Drush 12 (possible earlier versions). Closing.

🇺🇸United States paulmckibben Atlanta, GA

I don't see any changes in the issue fork, but I have updated the README as suggested.

🇺🇸United States paulmckibben Atlanta, GA

@wolffereast Thanks for the patch! This is committed to 7.x-2.x.

🇺🇸United States paulmckibben Atlanta, GA

@DamienMcKenna Thanks for the patch. I have committed it to the 7.x-2.x branch.

🇺🇸United States paulmckibben Atlanta, GA

@DamienMcKenna, thanks for the patch, and sorry for the long wait. I have committed your patch to the 7.x-2.x branch.

🇺🇸United States paulmckibben Atlanta, GA

As useful as tests would be, with Drupal 7 end of life approaching, I don't have any plans to address this. If you'd like to contribute tests I'll be glad to merge them in. Thanks!

🇺🇸United States paulmckibben Atlanta, GA

Closing this issue due to its age. Feel free to reopen if this is still a current question for you.

🇺🇸United States paulmckibben Atlanta, GA

Making sure @drumm gets credit.

🇺🇸United States paulmckibben Atlanta, GA

A fix for this has been pushed to the 7.x-2.x branch.

🇺🇸United States paulmckibben Atlanta, GA

I created the above merge request. The infinite loop in attachBehaviors was due to not passing the context and settings parameters to the timeout callback, causing attachBehaviors to be invoked on the document element infinitely.

🇺🇸United States paulmckibben Atlanta, GA

I was able to reproduce this issue in version 6.2.2. I'll try @kevinsiji 's fix and report back.

🇺🇸United States paulmckibben Atlanta, GA

I have, in the past, supported a D7 site on Windows/IIS, and I've had inquiries about the possibility of running a corporate-internal D10 site on Windows. This is to say, there are still a few organizations out there who run Windows server networks internally, but still have an interest in using Drupal and other open source software.

I think it's fine to drop IIS support. However, I think we need to distinguish that from "Windows" support. For example (and I haven't tried this), I've read it should be possible to use a Docker-based solution to run Linux with Apache or Nginx and PHP on a Windows host. In such a scenario, Drupal may not be relying directly on Windows, but it would still be running on a Windows server.

  • What I'd like to see is a conclusive statement about what is or is not being supported by the security team. Is it IIS, or anything where Windows is involved? I'm seeing allusions to both of these in the previous comments.
  • If possible, if anyone has experience/expertise and can share, I'd like to see the documentation updated for "Best practices when running Drupal on Windows using Docker" or something along those lines. If my situation ever gets beyond inquiries and I have to figure it out, I'd be glad to contribute to such documentation, but I hope I'm not the first to even think about trying this.
  • Overall, I think I am more concerned about community support rather than security support, if that distinction makes sense. I know Windows is an edge case, and I'm unconcerned if the DST needs to exclude IIS and the Windows OS from security support. In a Docker-based scenario, chances are, any security issue would be more related to the architecture within Docker rather than the host anyway. What I would like to see is sharing of best practices by those who have had to support this use case. And to avoid a blanket "We don't support Windows" statement, but a more nuanced, "If you must use Windows, here is an approach that works."

Thanks to the DST, core maintainers, and everyone else who has put so much thought into this. I'm writing this to share my limited experience with this scenario. If there is more I can do to help, let me know.

🇺🇸United States paulmckibben Atlanta, GA

Ah, there hasn't been a new release of the Profile module since the fix for 📌 Some menu blocks can not be loaded by id Fixed 4 months ago. This is the same issue. Applying the patch from that issue fixes the problem for me.

Plea to the maintainers: can a new version of Profile be released, please, to prevent further confusion on this issue? Thanks!

🇺🇸United States paulmckibben Atlanta, GA

I'm seeing this as well, despite the fix for similar issue 🐛 Drupal 10.1: fatal error on account creation Fixed .

🇺🇸United States paulmckibben Atlanta, GA

Agreed, the merge request works well for me. Marking this RTBC.

🇺🇸United States paulmckibben Atlanta, GA

Rerolled #6 against the 10.1.x branch. This is merely a workaround.

🇺🇸United States paulmckibben Atlanta, GA

Hmmm... I'm at a loss on how to fix the test failures. I have no problems using this patch on a live site. Any suggestions?

🇺🇸United States paulmckibben Atlanta, GA

Hi,

I'd like to reopen this, as "alternateName" is currently not available for the WebSite schema in version 3.0.1 of schema_metatag.

Thanks!

🇺🇸United States paulmckibben Atlanta, GA

Looks good to me. I'll commit this.

🇺🇸United States paulmckibben Atlanta, GA

The patch in #6 works for me. @Liam Morland, thank you!

🇺🇸United States paulmckibben Atlanta, GA

@srdtwc - thanks for your feedback. I have other out-of-date modules, and they did not get updated. But maybe I am misunderstanding what you mean.

🇺🇸United States paulmckibben Atlanta, GA

Thanks @aprice42. Here's a one-line version that also works:
composer require drupal/mailchimp:^2.2.2 --update-with-all-dependencies

🇺🇸United States paulmckibben Atlanta, GA

@DamienMcKenna, yes, 3.x would be fine. And I also appreciate your eyes on the documentation. Thank you.

🇺🇸United States paulmckibben Atlanta, GA

Damien, thanks for clarifying that the issue has to do with automatically downloading the file using drush make. That was not clear from the issue description or comment history.

I'm doing my best to support the Drupal 7 version on a best effort basis, so I may need further help. I see that we need to support a download mechanism using drush make. However, I'd also like to avoid hard-coding a version number if possible. Is there a way we can do that?

🇺🇸United States paulmckibben Atlanta, GA

Hi @bcobin,
Can you please give me the specific steps to replicate this issue on a brand-new Drupal 10 installation? For example:
- Step 1: install Drupal 10.
- Step 2: install colorbox.
- Steps 3-N: each incremental step until the issue is reproduced.

I need to reproduce the problem myself, in a local environment, so I can clearly verify whether a proposed patch actually fixes the issue.

Thank you.

🇺🇸United States paulmckibben Atlanta, GA

Sorry, I don't exactly understand your use case. Can you list the steps to reproduce your issue?

🇺🇸United States paulmckibben Atlanta, GA

Can you please list the steps to reproduce your issue? Thank you.

🇺🇸United States paulmckibben Atlanta, GA

Can you please add specific steps to reproduce the issue? Thank you.

🇺🇸United States paulmckibben Atlanta, GA

1. Can you please add steps to reproduce?
2. Does this bug still exist in the 2.0.x version of the module?

Thank you.

🇺🇸United States paulmckibben Atlanta, GA

Thanks for the patch and reviews, everyone!

🇺🇸United States paulmckibben Atlanta, GA

I will move this to the current branch and push a commit shortly.

🇺🇸United States paulmckibben Atlanta, GA

For security reasons, you should not install the entire contents of the DOMPurify archive. You should only install purify.min.js, in sites/all/libraries/DOMPurify/purify.min.js.

🇺🇸United States paulmckibben Atlanta, GA

I fixed some typos at the end of README.md, but otherwise it looks good. Merging. Thanks to all involved!

🇺🇸United States paulmckibben Atlanta, GA

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

🇺🇸United States paulmckibben Atlanta, GA

Confirming that by following @rfay's advice in https://github.com/ddev/ddev/issues/4413, I was able to resolve this problem for myself.

This applies to local environments on ddev/MacOS/Colima:
Type: colima restart -e
And then edit the configuration file to modify the dns to use 8.8.8.8 and 1.1.1.1:

# Network configurations for the virtual machine.
network:
  # Assign reachable IP address to the virtual machine.
  # NOTE: this is currently macOS only and ignored on Linux.
  # Default: false
  address: false

  # Custom DNS resolvers for the virtual machine.
  #
  # EXAMPLE
  # dns: [8.8.8.8, 1.1.1.1]
  #
  # Default: []
  dns:
    - 8.8.8.8
    - 1.1.1.1
🇺🇸United States paulmckibben Atlanta, GA

In further digging, my particular problem may be with the Colima container that ddev uses. See https://github.com/ddev/ddev/issues/4413

In general, my problem seems to be "api.sendgrid.com" can't be resolved by my local ddev container.

🇺🇸United States paulmckibben Atlanta, GA

I'm seeing this error as well, but only in my local (ddev) environment. Once I push to a Pantheon dev environment, it works fine. It would be helpful if this could work in a local environment for easier testing.

🇺🇸United States paulmckibben Atlanta, GA

@Berdir and @catch, thanks for the patch and review!

🇺🇸United States paulmckibben Atlanta, GA

Looks good to me. Thanks for the patch, @czigor!

🇺🇸United States paulmckibben Atlanta, GA

@diqidoc, thank you for weighing in on this.

There is indeed already an issue for the UUID concern, 🐛 Using Domain Config UI creates *.yml files with duplicate UUIDs. Needs work .

To clarify what I meant by configuration edit forms, I probably should have written configuration entity edit forms, i.e. the form that is specified in the configuration entity annotation. In the case of metatag, the entity is Drupal\metatag\Entity\MetatagDefaults. This class has annotations that define form to add or edit the MetatagDefaults entity, among other definitions. For MetatagDefaults, the form is Drupal\metatag\Form\MetatagDefaultsForm.

For reasons I have yet to be able to figure out, the "entity" member of MetatagDefaultsForm gets set to the MetatagDefaults configuration entity without overrides. I am pretty sure I read somewhere that this is the default/desired behavior for entity forms, but of course, I can't find it now. :)

The patch loads the entity with overrides and populates the form with overridden values.

The patch overrides the default behavior, yes.
But without the patch:
- When you load the form, it loads the global settings.
- When you save the form, it saves the domain-specific settings.

That is a bug for sure. This patch at least makes the load and save behavior consistent by changing the first bullet to loading the domain-specific settings.

Thanks for the link to the PreviousNext article. That article references 🐛 There is no indication on configuration forms if there are overridden values Needs work , which definitely pertains to this issue.

🇺🇸United States paulmckibben Atlanta, GA

As best I can tell, the UUID issue is a red herring. Having unique UUIDs didn't fix the problem for me.

It appears that configuration edit forms, by default, ignore overrides. The domain_config module adds module overrides to the configuration behind the metatags_default configuration entity. The only fix I could come up with is for the entity form to buck the default behavior and actually take overrides into account.

Patch attached. I'm interested in others' thoughts on this approach. It addresses the issue, but I don't know if departure from the default behavior is the right answer.

🇺🇸United States paulmckibben Atlanta, GA

Or, maybe it does have something to do with the UUID after all. There is an open issue in the domain project, 🐛 Using Domain Config UI creates *.yml files with duplicate UUIDs. Needs work .

🇺🇸United States paulmckibben Atlanta, GA

I can confirm what @Lukas von Blarer is seeing. For each domain, for the same configuration, the UUID is the same.

Example:
domain.config.domain_a.metatag.metatag_defaults.node__article.yml
domain.config.domain_b.metatag.metatag_defaults.node__article.yml
metatag.metatag_defaults.node__article.yml

All three have the same UUID.

However, this is true for other configuration files outside of metatag, e.g. system.site.yml. So I don't think this is the root cause of the problem.

🇺🇸United States paulmckibben Atlanta, GA

Also, the problem only happens with the mini-pager. The full pager shows the correct number of rows.

🇺🇸United States paulmckibben Atlanta, GA

Also, for what it's worth, the change in MR 1865 in #3 did not fix the problem for me.

🇺🇸United States paulmckibben Atlanta, GA

I was able to reproduce this issue as follows:

1. Create a block view of articles, where there are in excess of 200 published articles.
2. Use a mini pager, and display 10 articles at a time.

The result:
- on the first page of results, I see view.total_rows = 11.
- on the second page of results, I see view.total_rows = 21.
- and keep incrementing by 10 on each subsequent page.

Expected result: view.total_rows should always reflect the total number of rows for all pages, which is in excess of 200.

🇺🇸United States paulmckibben Atlanta, GA

Thanks, this provides further data for 🐛 DOMPurify library path - inconsistent capitalization Needs work . I'm going to mark this one as duplicate. Feel free to follow 🐛 DOMPurify library path - inconsistent capitalization Needs work for an eventual fix.

🇺🇸United States paulmckibben Atlanta, GA

@BEGRAFX, does #3314730-8: DOMPurify library path - inconsistent capitalization help? i.e. does changing the capitalization to /libraries/dompurify/dist fix the issue for you?

I've been tracking this problem in 🐛 DOMPurify library path - inconsistent capitalization Needs work . Something in the Drupal 10 compatibility update seems to have affected the library path. It's not reproducible for everybody (i.e. both DOMPurify and dompurify work for me in a docker environment), but it does seem to happen for many people.

If changing the directory name to "dompurify" fixes the issue for you, that provides further evidence of the problem in 🐛 DOMPurify library path - inconsistent capitalization Needs work .

🇺🇸United States paulmckibben Atlanta, GA

The workaround patch needed a reroll for 9.5.x. New workaround patch attached.

Production build 0.69.0 2024