Atlanta, GA
Account created on 18 April 2008, almost 17 years ago
#

Merge Requests

Recent comments

🇺🇸United States paulmckibben Atlanta, GA

If you need to automate this workaround in composer, you can do so with a post-install command:

    "scripts": {
        "post-install-cmd": [
            "rm -rf web/profiles/drupal_cms_installer/cache"
        ]
    }
🇺🇸United States paulmckibben Atlanta, GA

@giordy,

As I explained before, it is not sufficient to merely insert an image in the body field and expect colorbox to work. That has been true since Drupal 8. The javascript library is only loaded under specific circumstances.

On your Drupal 10 site, you must have done something, somewhere, to cause the colorbox library to be loaded. Perhaps you have defined a field that uses the Colorbox image formatter in "Manage Display".

I and others have been able to make Colorbox work in Drupal 11 without a problem, as long the content type is properly configured with an image field that uses the Colorbox formatter. Alternatively, you can attach the libraries yourself if you are using a custom theme.

I am going to change this to a documentation issue, with an action item to provide more specific setup instructions.

🇺🇸United States paulmckibben Atlanta, GA

@giordy,

As I explained before, it is not enough to merely insert an image in the body field and expect colorbox to work.

You have to do something to make the Colorbox library load, such as attaching the colorbox/colorbox library in your theme, or adding a field with a display mode that uses colorbox.

Clearly, you have done something on your 10.3 site that has caused the colorbox library to load. This hasn't been done on the 11.0.5 site.

Please see my suggestions in comment #2.

🇺🇸United States paulmckibben Atlanta, GA

@giordy and @junk_box, I'll be glad to help further if you will please provide a set of steps for me to reproduce your problem using a clean Drupal installation.

i.e.

Step 1: Install Drupal 11.0.5 with the standard profile
Step 2: Install the colorbox module using composer
Step 3: Install the colorbox library
Steps 4..n: Each additional step for me to replicate the exact setup that is failing for you, up to the point where I click on the link that should display in a colorbox and it fails to do so.

Thanks,
Paul

🇺🇸United States paulmckibben Atlanta, GA

2. I put in the subtheme file gmpe.info.yml:

libraries:
- colorbox
- dompurify
but it doesn't work.

Your theme info.yml file needs to have:

libraries:
- colorbox/colorbox
- colorbox/dompurify
🇺🇸United States paulmckibben Atlanta, GA

Hi @scslweb, thanks for the report.

I'm unable to reproduce this issue with empty alt text passing a NULL to Xss::filter(). When I have empty alt text, the string "" is passed as designed.

Can you provide a set of steps to reproduce this issue on a clean Drupal install?

Thanks!

🇺🇸United States paulmckibben Atlanta, GA

Hi @ismaelromero, thank you for explaining that the colorbox library needs to be installed in order for colorbox to work.

@phelix, does this solve your issue? If not, please provide a reliable set of steps on a clean installation of Drupal to reproduce the issue, including:

  1. Install Drupal [core version]
  2. Install colorbox [module version]
  3. Install colorbox library
  4. Install dompurify library (if you need it)
  5. Any additional setup steps you performed (e.g. setting display mode of your image field)

If you can provide those steps and I can reproduce the issue, I'll be glad to help. Thanks!

🇺🇸United States paulmckibben Atlanta, GA

Reducing priority and changing to support request for now.

@candelas and @mr.york, if you can provide a reliable set of steps on a clean installation of Drupal to reproduce the issue, starting with:

  1. Install Drupal [core version]
  2. Install colorbox [module version]
  3. Other configuration steps necessary to demonstrate colorbox is working
  4. Upgrade colorbox to [new version]
  5. Show that the same steps that worked before don't work any more.

If you can provide those steps and I can reproduce the issue, I'll be glad to help. Thanks!

🇺🇸United States paulmckibben Atlanta, GA

It's possible that colorbox.js is not loaded on your page, especially if you are not using anything that would actually load the colorbox Javascript.

If you're using an image filed with the Colorbox display formatter, then the javascript will be loaded. But if you're not doing that and are only relying on the colorbox class, that by itself will not load the javascript library.

If you are able to edit your theme, there are a couple ways to make sure the library gets loaded (it's the "colorbox/colorbox" library):

1. If you want colorbox to be present all the time on all pages, update your [theme].info.yml file by adding colorbox/colorbox to your libraries (and add colorbox/dompurify if you need that too):

libraries:
  - colorbox/colorbox
  - colorbox/dompurify

2. If it's only on certain templates where you will need it, you can edit the relevant twig files, e.g. page.html.twig, by placing the necessary attach calls at the top:

{{ attach("colorbox/colorbox") }}
{{ attach("colorboxy/dompurify") }}

3. There are other ways to attach javascript libraries, e.g. in theme preprocessing, so if you're familiar, use the approach that works for you.

🇺🇸United States paulmckibben Atlanta, GA

The MR fixes the error for me. @davps, thanks for the fix!

🇺🇸United States paulmckibben Atlanta, GA

@dydave, thanks for letting me know about this. I removed the logo from the project page images.

🇺🇸United States paulmckibben Atlanta, GA

Updated patch attached with one more attribute added to the main openlayers.module file.

🇺🇸United States paulmckibben Atlanta, GA

I'm seeing this error on a site running Drupal 10.3.4 when I try to look at revisions for a taxonomy term. The revision in question is returning null for getRevisionCreationTime(), and that is leading to the exception described in this issue. I am not able to reproduce it on a clean Drupal 10 install, which leads me to suspect that the problem of the null revision creation time might be an artifact of upgrading this site over the years from Drupal 8 to 9 to 10.

I don't know whether this is a one-off or some obscure upgrade bug, so I don't think it's worth reopening the issue. Documenting what I found in case it's helpful to somebody else.

🇺🇸United States paulmckibben Atlanta, GA

Hi @DYDave,

Thank you for all the work you have done to fix the ESLint errors and refactor the code for modern javascript.
I left some comments in the merge request, but they are all the same (different files): when attaching behaviors, is adding the requirement for context === document going to break colorbox for ajax-loaded content? e.g. in a multi-page view where results are loaded using ajax.

I did not try to test this, and it's possible I missed something where this scenario could still work. Please let me know what you think.

Thanks!

🇺🇸United States paulmckibben Atlanta, GA

Thanks, everyone! This has been merged.

🇺🇸United States paulmckibben Atlanta, GA

Closing this as outdated since it was reported against a now-unsupported version of the module and nobody else has reported the issue in recent versions. Feel free to reopen and update the version if you experience the issue in a currently-supported version of the module.

🇺🇸United States paulmckibben Atlanta, GA

Hi @sanvibcn20,

Can you please provide a detailed list of steps to reproduce this issue? Otherwise, I'm not sure how to help you.

Thank you.

🇺🇸United States paulmckibben Atlanta, GA

The patch from #38 works great for me in Drupal 10.3.2. Thanks @szeidler!

🇺🇸United States paulmckibben Atlanta, GA

Folks, I'll defer to your expertise here, as my graphics skills are pretty basic. If somebody would like to offer a new logo with the gradient changes that @lostcarpark suggests, I'm happy to commit it! Otherwise, if we want to continue with the existing logo, I'll be happy to commit that instead.

🇺🇸United States paulmckibben Atlanta, GA

Thanks to everyone for their work on this. This is now committed.

🇺🇸United States paulmckibben Atlanta, GA

@DYDave, thanks for the patch! Merged MR 38.

🇺🇸United States paulmckibben Atlanta, GA

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

🇺🇸United States paulmckibben Atlanta, GA

Thanks all, I have merged MR 40.

🇺🇸United States paulmckibben Atlanta, GA

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

🇺🇸United States paulmckibben Atlanta, GA

@DYDave, thanks for cleaning this up, and @dww, thanks for reviewing. I have merged this MR.

🇺🇸United States paulmckibben Atlanta, GA

@ivnish, thank you for catching this problem and for the patch! I've merged your MR.

🇺🇸United States paulmckibben Atlanta, GA

Hi,

I am unable to reproduce the original issue using the "steps to reproduce" you listed. Therefore, I'm not able to verify the fix. Can you please provide an explicit set of steps necessary to reproduce the issue?

Thank you.

🇺🇸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

Thanks, this is a duplicate of 📌 Don't use static t() function when class method is available Fixed which was fixed today.

🇺🇸United States paulmckibben Atlanta, GA

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

Production build 0.71.5 2024