San Francisco, CA & Berlin, DE
Account created on 15 August 2007, about 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@nishargshah The repository is here: https://git.drupalcode.org/project/drupal/-/tree/11.x

You'll want to create an issue fork. The guide for doing so is here: https://www.drupal.org/community/contributor-guide/task/create-a-merge-r...

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I had to downgrade cweagans/composer-patches to ^1 to get this patch to apply. However, I can confirm that with this applied, my above scenario with canvas-demo is resolved 🎉.

I've only had limited time to test against rc1 / this patch, but so far everything appears to be working as expected.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Bumping the priority as this is now breaking the installation of new Drupal demo sites. There's chatter about this on the Slack:

Reproduction steps:

  • Setup the `canvas-demo` app
  • Update packages in Composer (Core to 11.2.5, canvas to rc1)
  • Attempt to do a `ddev drush site:install`
  • See fatal error.
🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Our team has also been successfully using content_export_csv with this patch in D11 for a few months now. +1

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

The v3 branch has not had a commit in more than 2.5 years. I think it is probably safe to say that it isn't being actively worked on and unlikely to be released anytime soon.

I was just coming to the issue queue to check on the module's progress myself :)

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

While updating the library, it might be worth looking at the other closed PRs and evaluating them for inclusion.

For example, this alleged XSS vulnerability has a PR: https://github.com/jackmoore/colorbox/pull/910 (Note: this security report is pretty thin, so it's unclear to me how real this is or not)

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty created an issue.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE
🇺🇸United States kmonty San Francisco, CA & Berlin, DE
🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Suggesting this be re-opened to backport the fix to 4.1.x, as this impacting Drupal 10.5 as well.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@joseph.olstad It's worth noting that these issues are not fixed in 2.x. The merges were only against the exiting 1.x D10 branch. Those issues might have to be re-opened so they can also be merged against the D11-compatible 2.x branch.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Confirming comment #51. If you are Core 11.1.x, you cannot install either 2.2.7 or 2.3.0.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Is this justifiable for a backport to 10.4 as well?

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Small suggestion for the future: when releasing a new minor version of a module, could you leave the old version as supported for a few weeks to give people time to update? When the old versions are immediately labeled as unsupported when a new minor release is made, it does not give maintainers an opportunity to update to the supported version.

For people with Update Status module enabled, it can cause confusion / concern with stakeholders that believe abandoned / insecure modules are in-use on their websites.

Thanks for all the hard work on Gin!

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

As mentioned in #29, you must apply so many patches to even use this module right now. Unfortunately, with all the patch conflicts in composer, it is borderline impossible to run v2. Honestly, I'd almost suggest they delist the releases as supported at this rate.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Closing this. The application in question was still on core 10.3. We upgraded to 10.4 and this issue resolved itself.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

The patch didn't apply against 2.1.0, so I opened an issue fork that is compatible. I'm not convinced about this patch's approach, but people who have a better understanding of Stable should know better.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

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

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I'm spinning off the composer issue into its own item here 🐛 Webform version constraints in TMGMT's Composer file are breaking builds Active

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty changed the visibility of the branch 3097660-add-layout-builder to active.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Our team found ourselves wanting to use both this patch and the patch from Cloning a node with existing translations, clones all translations of the node Active . The problem is, 3049127 introduced a bug into this patch.

We couldn't figure out a really 'clean' way to reconcile fixing the two patches while adhering to issue encapsulation best practices.

As a result, we'll upload the changes we made to this MR here for other users / future reference for the community. But we didn't want to put undesired changes back into this MR directly.

Essentially, here are our changes.

Line 156 was:
$section = $this->cloneLayoutSection($section);
now:
$section = $this->cloneLayoutSection($section, $entity->language()->getId());

Line 171 was:
public function cloneLayoutSection(Section $section) {
now:
public function cloneLayoutSection(Section $section, string $langcode) {

At line 201, we inserted the following conditional logic:

        if ($this->getConfigSettings('skip_translations')
          && $block_content instanceof TranslatableInterface
          && $block_content->isTranslatable()
        ) {
          // Loop through the unwanted translations and remove them.
          foreach ($cloned_block_content->getTranslationLanguages() as $lang => $language) {
            if ($lang !== $cloned_block_content->language()->getId() && $lang !== $langcode) {
              $cloned_block_content->removeTranslation($lang);
            }
          }

          // Save the block.
          $cloned_block_content->save();
        }
🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Noting this is probably a dupe. There's already a patch for this which we're using on production 📌 Automated Drupal 11 compatibility fixes for content_export_csv Needs review

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

This is still a WIP. The offcanvas library still has some issues I need to iron out. I'm planning on picking this back up this week.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

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

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I mean you could have clicked on my profile to see I've been in the community for 17+ years and maintained multiple modules and a profile. But yeah I'm just a spammer in your eyes.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Rolling back my lint fixes at the request of the maintainer.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Apologies, I was just trying to get Add a Soft Limit option Active to pass.

I'll pause contributing to not waste your time.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Fixed all outstanding build issues.

Resolved a PHPCS failure unrelated to this ticket in 🐛 Resolve PHPCS issues in 7.1.x-dev Active

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Noting that merging this will get Add a Soft Limit option Active passing.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty created an issue.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

The ESLint job is now passing. If I have more time at ddd2025, I'll also strip out jQuery in favor of vanilla ECMA 2020 JS.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

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

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@Christian.wiedemann I only briefly scanned this PR, as I was curious was gin_lb_plus was. I noticed some quick things

1. `core/jquery.once` no longer exists and https://www.drupal.org/project/once should be used.
2. It looks like the PHP won't pass linting, but perhaps that isn't a concern for this project.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Closing this as a dupe of 📌 Drupal 11 compatibility Active

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@megachriz The module should probably drop support for Commonmark v1 given it is insecure / out of support. But I agree that is best handled in Add support for Commonmark v2 Active

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I would suggest removing the `version` attribute completely. Many other projects do not have one. See Paragraphs, for example: https://git.drupalcode.org/project/paragraphs/-/blob/8.x-1.x/composer.js...

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Bumping this as critical, as the module cannot be installed anymore.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE
🇺🇸United States kmonty San Francisco, CA & Berlin, DE

The scope of this PR is unmanagable in my opinion. For example, the first file in the PR is js/contextual.js. That file has nothing to do with D11 compatibility.

That issue is addressed in 🐛 Contextual links for translation are removed by core Active .

The PR in this issue goes against standard issue encapsulation practices and puts roadblocks to the community to participate. It's not even clear to me what should be tested here, as this has turned into a meta issue, rather than a D11 compatibility issue.

Humble suggestion: close issues with merges that are considered blockers first?

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I think it would be helpful to create a 2.x branch, even if it is in alpha state, so folks can test it more. Given the scope of published breaking changes, I don't see a reason that gin_lb wouldn't be compatible as-is.

I was going to spend some time testing this today, as there some issues with the sticky nav and our Layout Builder setup that we are encountering, but we cannot even upgrade to the latest release of Gin without manually hacking our composer lockfile.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Also +1 RTBC. My code concern was addressed. Given this project does not have automated tests, there does not appear to be any additional work that needs to be done.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty created an issue.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Won't this patch cause a regression with RTL languages?

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@bramdriesen It isn't self-contained. You can see it in the core Webform module's Composer libraries file https://git.drupalcode.org/project/webform/-/blob/6.3.x/composer.librari...

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

+1 for this. Good reasons to do this:

  1. Stewardship: The maintainer doesn't appear to care about downstream impacts and security exploits that changing their username could create.
  2. Technical Debt: This is written in jQuery, which the Drupal ecosystem is trying to move away from.
  3. Maintenance: It hasn't received an update in more than 4 years. The "coming soon" v2 was promised 10 years ago and still hasn't been released.
  4. Criticality: This module only provides UI niceties--hardly mandatory for Webform itself--that largely do not require a JS library in the modern browser ecosystem.
🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Not to hijack this, but this would be a good opportunity to upgrade to 1.0.3 as well. The release being used with iCheck is from 2014.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

It looks like this was already resolved in an unrelated Gitlab migration issue 📌 Gitlab CI yml & Drupal 11 support Fixed : https://git.drupalcode.org/project/block_list_override/-/commit/c4725035...

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

If anyone else is looking into why Layout Builder broke after this was merged in, you can find a patch here 🐛 Uncaught TypeError in sidebar.js after updating to the latest Gin theme RTBC

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Note: our team similarly encountered this issue. This patch resolved it.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

It looks like this was merged into the 1.x branch in error, so that branch won't be able to be updated without this being pulled out.

2.x RELEASE branch was just 1.x with this PR merged in. The 2.x DEV branch is years out of date.

Might be a good opportunity to move to semvar while cleaning this up?

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

We just encountered the same issue after updating the latest RC of Gin.

Actions:
1. Created a PR for the 3.0.x branch that @andybroomfield created.
2. Patched the 2.0.x branch, as that is the current supported version of the module.
3. Confirmed this resolved our team's error.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty changed the visibility of the branch 2.0.x to hidden.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

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

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty created an issue.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

@Rajeshreeputra Thanks for changing the status of `2.0.3`

I just wanted to nudge that hopefully Focal Point makes this a pattern for releases in the future :)

Thanks for maintaining a great module!

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

No patch here, so this can't be RTBC

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I just wanted to +1 this.

Other projects will leave releases like `2.0.3` as supported (but not recommended) so it doesn't flood Admin UIs with a scary red banner asserting a module is unsupported.

This can inject stress into teams, especially from non-technical stakeholders who are security-focused.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Great call, @anybody! I just looked at the 2.x branch and jQuery isn't used / required as a library. I'll leave it up to the maintainers to close this as outdated, but it strikes me as something that could be.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I think it makes sense to remove the dependency. Core can require it, but the module itself doesn't need to. Eventually the goal is to remove the jQuery dependency from core (as I understand it), so it seems reasonable to get ahead of that.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I found a bug with the latest patch with Webform Submissions. It causes a PHP error Recursive rendering attempt aborted for webform_submission1html. In progress: Array ( [webform_submission1html] => webform_submission1html ).

Reproduction steps:

1. Be on Core 10.2.4 and the latest stable release of Webform. Apply the patch as of the 15.03.2024 commits.
2. Setup a basic Webform, submit a form (will be submission ID #1)
3. Go to /admin/structure/webform/submissions/manage and go to View the submission you just put in.
4. Instead of seeing a list of fields that you filled in, you just see the "Submission information"
5. If you have dblog enabled, you can see the PHP error in the latest log messages

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Our QA team tested the patch file (not the MR) which removes the library and found no issues.

+1 for removing the library entirely, as IE 11 has been at end of support for near two years now.

+1 RTBC

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I think this should be higher than `normal`, as it's a security threat.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

kmonty created an issue.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I'd say this is `Needs Work`, as this is an `ugly, temporary solution`.

At the very least, it should remove the dependency on jQuery.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Several people reporting issues. Moving back to Needs Work.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

The new patch is just for my suggested changes against the 8.x branch and not updating the existing patch. It also does not include eslint changes.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Minor change, but the `once` dependency should be included in the wrapper function.

So:

(function (Drupal, drupalSettings) {
[...]
})(Drupal, drupalSettings);

becomes:

((Drupal, drupalSettings, once) => {
[...]
})(Drupal, drupalSettings, once);

Also, this should be run through eslint, as it doesn't match Drupal's Prettier rules (single quotes instead of double quotes, no unnamed funcs, incomplete code comments // If no error messages exist, the etc.)

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I won't go as far as modifying the child issues, but calling out we found a breaking regression that was implemented in beta7. 🐛 Regression > Patch for "webform entity reference formatters not supporting translations" in 6.2.0-beta7 breaks Webform embeds within untranslated entities Active

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Here is a small patch we wrote to add a setting to enable the display of the signature file.

There are probably more elegant ways to write this, such as:

  1. Doing a file_exists check.
  2. Changing the logic to be disable_signature instead of enable_signature. As this patch is written now, a database update hook would be required before merging this into the branch. That said, it was a quick way for our team to solve this.

Hoping this helps folks for now. We might pick this back up to improve the patch if the maintainers seem interested in merging at some point + offer up guidance as to what their preferred implementation is. Cheers!

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

Our team is satisfied that this is sufficient to get us to be able to deploy the security release. I didn't look into if this needs tests or anything. Again, just prioritized getting today's security release out. Hope this helps folks!

Note: patch was written against `3.0.x-dev` but it applies cleanly to `3.1.0`

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I have a patch working on localdev. Going to have our QA team give it a test before posting here. Assigning to myself for now.

🇺🇸United States kmonty San Francisco, CA & Berlin, DE

I'm currently working on a patch based off the work done here: https://www.drupal.org/project/language_switcher_menu/issues/3348242 🐛 SA-CORE-2023-003 Fixed

Production build 0.71.5 2024