Account created on 20 June 2006, over 19 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States xjm
🇺🇸United States xjm

xjm created an issue.

🇺🇸United States xjm

Committed the hotfix to 11.x. Thanks!

🇺🇸United States xjm
🇺🇸United States xjm

xjm created an issue.

🇺🇸United States xjm

Snark aside, I think this would be a serious regression in the review process that would add work rather than reducing it and am strongly -1. I agree with the previous remarks from @catch and @phenaproxima.

There are many, many automated tooling improvements we can invest in that would reduce workload -- not to mention direct investments in human reviewers -- that would actually improve the review process.

AI hallucinations with serious misinformation are a serious issue I encounter regularly, and "seemingly plausible but actually seriously wrong" is a very accurate description of most LLM-generated code reviews I've seen.

🇺🇸United States xjm

Reminder: Dries Buytaert is the founder of WordPress and associated with the internet.

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

Compared this again to the original commit from the original issue. Confirmed the only differences between cherry-picking the commit and the MR are in the lockfile hash and core-recommended:

[ayrton:drupal | Tue 19:46:18] $ git diff 3561574-bump-versions-11.2.x --stat
 composer.lock                                      | 150 ++++++++++-----------
 composer/Metapackage/CoreRecommended/composer.json |  24 ++--
 2 files changed, 85 insertions(+), 89 deletions(-)

I then reviewed those specific changes with:

[ayrton:drupal | Tue 19:46:37] $ git diff 3561574-bump-versions-11.2.x --color-words="."

I verified that the only differences are in lockfile hashes and timestamps, patch version increases to individual Symfony patches, and in one case an addition to the release managers listed as supporting a package in the lockfile. So those are all appropriate changes in a patch release.

Finally, I ran a composer validate --check-lock just to make sure the hashes etc. were correct, and they are, so RTBCing.

🇺🇸United States xjm
🇺🇸United States xjm

Edited my above comment to mention that we also bumped the priority of the open issues based on our best understanding of our severity. I should have also promoted this meta itself at the time, so doing that now.

It's also always helpful to have accessibility maintainers or experts in the field give feedback on the relative severity of the individual issues, and of course we could use help fixing them in the best way possible as well. Most accessibility bugs are eligible for backport even in patch releases, because the disruption to sites is outweighed by the value of resolving the accessibility bug. Resolving the more serious accessibility issues is our top priority for Navigation going forward.

🇺🇸United States xjm

Also no longer in a novice status. Thanks!

🇺🇸United States xjm

At DrupalCon Nara I told someone verbally this needed framework manager review, which somehow never made it back onto the issue, but looks like @alexpott found it anyway which is good.

🇺🇸United States xjm

I went ahead and committed the 10.5.x MR despite the normal backport policy. In some ways, the 10.5.x hotfix release is more urgent, since 10.5.x sites are more in need of security-only dependency management and less likely to want to simply update Symfony to the latest release for the sake of updating it.

NW for the 11.2.x fix to remove the minor updates of those two components which must be slipping by indirect constraints or something. (The patch-level updates are fine.)

🇺🇸United States xjm

Reassuringly, the branch I created by cherry-picking the original commit to 10.5.x differs from the new MR that was created above by only:

diff --git a/composer.lock b/composer.lock
index d8cf1dc814e..339476ca045 100644
--- a/composer.lock
+++ b/composer.lock
@@ -10211,5 +10211,5 @@
     "platform-overrides": {
         "php": "8.1.0"
     },
-    "plugin-api-version": "2.6.0"
+    "plugin-api-version": "2.9.0"
 }

I made a similar cherry-pick on 11.2 for comparison. That diff includes different versions of numerous Symfony packages than what we had before. To some extent, this is to be expected; however, I did notice that we're upgrading the minor version from 7.3.0 to 7.4.0 of symfony-string and symfony/var-exporter. That shouldn't be happening in a patch release.

🇺🇸United States xjm

Sorry I have no idea what diff I was looking at that had only lockfile changes and nothing else. Please disregard above several comments (aside from the 10.4.x cherry-pick which is fine).

🇺🇸United States xjm

It's interesting that no tests are failing despite the dependencies in composer.lock not matching what's in the metapackages in the above MRs.

Probably we should just try to follow the documented steps for updating core Composer dependencies for at least HttpFoundation.

🇺🇸United States xjm

I addressed 10.4.x by cherry-picking the original fix from 📌 Update Symfony to 7.3.7 / 6.4.29 Active and applying it again. That worked since there were no commits on 10.4.x since the release and none of the other changes in the security release touched the metapackages or lockfile.

🇺🇸United States xjm

Sorry, I think this is actually NW. There should be a corresponding increase in the constraint of http-foundation in core-recommended to the secure version. 10.6.x has this, and these branches also did prior to the merge commit eating it. See 📌 Update Symfony to 7.3.7 / 6.4.29 Active .

🇺🇸United States xjm

One thing that's interesting is that these are lockfile-only issues. Did we not increase constraints for the Symfony updates last month?

🇺🇸United States xjm

@catch and I discussed this hotfix. We agree it can be committed with the upgrade tests as a followup if the upgrade has been manually tested so that we confirm the issue is actually resolved. It should be tested both on a site where the update already broke and on a site that has not yet updated. Thanks!

🇺🇸United States xjm

I added credit for Rachel who also helped us a bit remotely with some of the organizational aspects, and for richo_au who contributed significantly to the event planning for the contribution day.

Thanks everyone!

🇺🇸United States xjm
🇺🇸United States xjm

10.6.x should be fine because it did not have a merge commit, just a forward-port.

However, I just checked and 11.2.9 has the same issue. It installs with 7.3.0, while 11.2.8 installs with 7.3.7. We definitely need an MR for 11.2.

(11.1 probably also has the issue, and we can ignore it like 10.4, but 11.2 needs the hotfix.)

🇺🇸United States xjm

@longwave correctly pointed out that 10.4.x is end-of-life in about 6 days, so we probably don't actually need to make a 10.4.x merge request because we're unlikely to create another 10.4 release unless some highly critical core security issue comes up that we would backport outside normal policy. Hopefully that doesn't happen. 😅 If that happens, or if we backport some other upgrade path fix to 10.4.x for some reason, then we can fix this, but otherwise 10.5.x itself is enough for now. Thanks!

🇺🇸United States xjm

Critical was an overreaction on my part because I was annoyed with myself; downgrading to major. While this is a serious bug from a release process standpoint, it is not a security issue. The site owner can still update the Symfony dependency easily -- it is only a patch-level update -- and Drupal itself isn't even vulnerable to the issue.

🇺🇸United States xjm

I just checked and while 10.4.x has not had a release since 10.4.9, it looks like it has the same lockfile merge issues that 10.5.x does. This means that the next time a 10.4.x security release is changed, it will revert the Symfony update. So we will need an MR for 10.4.x as well.

I think this is due to an issue with the security release tagging script naively assuming that security updates do not change the lockfile beyond the version constant. So while we resolved the new issue that came up with the new attempted process tags themselves during the security window, we did not resolve the second, similar, existing issue with the "Back to dev" commit.

In the longer term this might require a change to the tagging script in order to create proper "Back to dev" commits, or at least a manual workaround by the release manager to amend the commit prior to pushing branch endpoints after the window.

🇺🇸United States xjm

Pipeline failure was:

    1) Drupal\Tests\system\Functional\System\CronRunTest::testCronUI
    Cron does not run when saving the configuration form.
    Failed asserting that 1764887928 matches expected 1764887733.

Sounds random, so I requeued it.

🇺🇸United States xjm

This may have been due to an error during the tagging process that was corrected for the other branches but possibly not for 10.5. We should double-check to ensure that 10.4 doesn't have the same issue.

🇺🇸United States xjm

Added attendees to the contribution record. Thanks!

🇺🇸United States xjm

xjm created an issue.

🇺🇸United States xjm
🇺🇸United States xjm

Assuming the downgrade was a crosspost. :)

🇺🇸United States xjm

This kind of sort of should have been a stable blocker.

The documentation page should actually be provided at https://www.drupal.org/docs/develop/core-modules-and-themes/core-modules... now that the module is in core.

🇺🇸United States xjm

Navigation is stable!

🇺🇸United States xjm
🇺🇸United States xjm

📌 Register equivalent updates on site install Active would be helpful to fix before 11.3.0 and 10.6.0 if we can (backporting anything necessary to 11.2.x and 10.5.x patch releases as well as needed).

🇺🇸United States xjm
🇺🇸United States xjm
🇺🇸United States xjm
🇺🇸United States xjm

xjm created an issue.

🇺🇸United States xjm

greggles credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

xjm credited xjm .

🇺🇸United States xjm

I agree that we can remove the test changes BTW.

🇺🇸United States xjm
🇺🇸United States xjm

This morning @longwave and I discussed the possibility of simplifying the equivalent updates situation a bit now that our release cycle is more predictable. Currently, we have to remember each minor to backport an issue preventing downgrades from 10.6.1 to 11.3.0 or 11.2.x (etc.). As of 12.0.0-beta1, we will know whether 12.0 is equivalent to 11.4 or 11.5, and which version is equivalent to 12.1, etc. So we might be able to dynamically prevent equivalent updates that way.

🇺🇸United States xjm

Tagging for #6.

🇺🇸United States xjm

I think we should plan to address #6 for Drupal 11/12 in a more holistic way, and possibly consider backporting fixes for it to 11.2 at least. I don't think it should block this issue, though, as the most immediate concern is for people who run latest versions.

🇺🇸United States xjm

We also don't have the topic maintainer review tags for either of these topics so tagging with a handwavy alternative. 😅 Since it overlaps both areas, I guess we'd want signoff from both our candidates for the new topic, and @alexpott and @berdir and @mondrake, for how we categorize the issues.

🇺🇸United States xjm
🇺🇸United States xjm

I think this is actually a subset of the "Testing" topic. Which apparently does not have its own component, unlike the other topics. Possibly we were concerned about people assigning things to the component instead of adding the "Needs tests" tag.

It would need to be "continuous integration" instead of "CI" because we don't use abbreviations, but it'd still be better to have less specificity and fewer components. All the other topics have their own component.

🇺🇸United States xjm

We also had some prior art for role descriptions and desired skills for release managers in particular that we'd hoped to make into a recruitment blog post (which became less urgent when we recruited @quietone and then @longwave, respectively).

Not sure if the desired skills info belongs in the governance or not, but we do already have some info on leadership qualities we want for the Core Leadership Team in the general intro (although we may still call it "roles with commit access").

🇺🇸United States xjm

Updated the contribution record with the mentors' usernames (including correcting @a_ramos' username with the underscore). Thanks everyone for mentoring, including those who stepped up to help on the day of!

(If you helped mentor and were not credited, just let me know in DM. Thanks!)

🇺🇸United States xjm

We need the release note draft to include the various known issues that @ckrina and I started tagging, including the accessibility issues and other bugs, with a CTA to fix them so that Navigation can be the default in Standard in 11.4.

🇺🇸United States xjm

Drupal 11.1 is end-of-life in two weeks. It's recommended to update to Drupal 11.2.

Alternately, use drupal/core instead of drupal/core-recommended so that you can unpin from the old version of Symfony.

Note that Drupal is not vulnerable to the issue described in CVE-2025-64500, as mentioned in the release notes, so it's not correct to say it's not "fixing the security issue". For Drupal, there is no security issue.

🇺🇸United States xjm

Just moving some completed issues to the "Done" section of the IS.

🇺🇸United States xjm

@smustgrave inquired about whether this is still eligible to backport. I didn't review the issue closely, but in general fixes to the upgrade path are the sort of thing we fix in maintenance minors, so we could consider it for backport after beta1 or even in a patch release if it's otherwise non-disruptive.

🇺🇸United States xjm
🇺🇸United States xjm

We started a triage of the outstanding issues filed since 📌 New Toolbar Roadmap: Path to Beta & Stable Active had an out-of-date-ish IS. @ckrina, @lauriii, myself, and others credited here triaged all the major bugs, all the issues listed in the IS, etc., and found no blockers there. However, we did find some concerning issues that we started to tag as known issues to mention in the release notes (not yet added to the release notes drafts, but tagged as "11.3.0 release notes"). We did not quite finish the whole list -- will try to do so ASAP.

A couple of the bugs we noticed from the accessibility review ( 🌱 [PP1][PLAN] Accessibility review Postponed ) sounded like criticals to me, and under normal circumstances might have been raised as stable blockers. Given that nothing else is blocking stable and that Navigation is such a huge usability improvement, @ckrina and I proposed instead making these issues blockers for including Navigation in the Standard profile. So, I proposed a release management exception to allow Navigation to be marked as stable despite having these accessibility issues, listing them as known issues in the release notes, and a call-to-action for help to get them fixed either before 11.3.0 or in a patch release.

Issues that were especially concerning were:

If Navigation is experimental, these are major. If it is stable, issues with parts of the page not being accessible to keyboard navigation become critical. (I thought I had already promoted one of those to critical, but apparently Shinkasen wifi ate it.)

Also maybe:

  • 🐛 Mobile version of Navigation should have focus trap Active
  • 🐛 Submenu opened with hover does not close by Escape key when another menu item has focus Active
  • Full accessibility roadmap:

  • 🌱 [PP1][PLAN] Accessibility review Postponed
  • To be clear, I still think Navigation should be marked stable in 11.3 regardless. I also recognize that Navigation needs community help with getting these accessibility bugs fixed. But these issues should have been stable blockers and anything we can do to communicate that they're known bugs we care about fixing is valuable.

    There's also another 50 or so random issues I need to review in the queue and I'll try to do so in the next day or so.

    🇺🇸United States xjm

    Also, usernames not being 1:1 with d.o user profile URLs is a longstanding thing for many years not added here?

    🇺🇸United States xjm

    For core's release notes, I was thinking of just taking the first line that has the issue info, without the attribution information. Otherwise, it gets overwhelmingly long and outweighs the human-readable release notes at the top. (We already completely omit the "all changes" section from alpha1, for example.) So it might not even apply to core.

    I do agree that the mismatch between GitLab and d.o usernames is a pain, but this issue is just surfacing that, not the source of it.

    🇺🇸United States xjm

    Fantastic work on this!

    I'd also create separate, individual followup issues for each @todo, like the renderer reference.

    🇺🇸United States xjm

    xjm created an issue.

    🇺🇸United States xjm

    No one showed up to the second orientation, but we did manage to recruit some folks organically. :)

    🇺🇸United States xjm

    Committed LIVE at DrupalCon Nara! Thanks to everyone who helped contribute to this issue over the past for years.

    Cherry-picked to 11.3.x as a minor-safe string change.

    Thanks!

    🇺🇸United States xjm

    Thanks for the great work on this everyone! I think we've come up with the best solution.

    Now that we've settled on a proposed resolution, we can simplify the proposed resolution section to just say that several different proposals were considered, and to document the one we chose.

    I agree with @rduterte's assessment that the issue of inconsistency between single and double quotes already has its own issue, and is therefore best not managed here. Good scope management! (There are legitimate cases for double quotes when strings contain apostrophes, for example, etc. etc.... and all that discussion would derail this issue, which is why we move the discussion outside this scope.)

    I've made two very tiny suggestions on the merge request. If the contributors think these are good changes, then you can review them and mark the issue back to "Reviewed and tested by the community".

    🇺🇸United States xjm

    xjm created an issue.

    🇺🇸United States xjm

    Did not realize this issue already existed; thanks @catch for the pointer. I agree that "feat" should not be the default and that we should define our own "task" over "chore"; the spec allows for it.

    I think #14 is a good strategy. In the off-chance that we do commit from plans or support requests for some reason, those are fine as defaults and we can always override it on the CLI if we want.

    🇺🇸United States xjm

    The 11.2.x, 10.5.x, and 10.4.x MRs were committed as part of today's security releases, and I've committed 10.6.x just now.

    We still need an update for 11.x and 11.3.x once Symfony provides an updated beta for Symfony 7.4, but that will come naturally in a followup issue when we update the dependency as part of our normal release process.

    @catch, @longwave, and I agreed not to update 11.1.x to 7.3 (as per the 11.1.9 release notes ).

    Thanks everyone!

    🇺🇸United States xjm

    Excellent. Postponing.

    🇺🇸United States xjm
        1)
        Drupal\Tests\file\Functional\DownloadTest::testPrivateFileTransferWithoutPageCache
        Correctly denied access to a file when file_test sets the header to -1.
        Failed asserting that 200 is identical to 403.
    🇺🇸United States xjm

    xjm changed the visibility of the branch 3557393-update-symfony-11.1.x to hidden.

    🇺🇸United States xjm

    We agreed not to update 11.1.x to Symfony 7.3, so I am going to hide that MR.

    🇺🇸United States xjm

    Additionally 🐛 [random test failure] ImageUrlProviderTest::testResize Active for 10.4.x.

    🇺🇸United States xjm

    For 11.1.x:

    git cherry-pick -x 597203c24f0ec7e0f7ff15d0c1626b0cc6c7371e
    git cherry-pick -x e38a58cccfd60e1b301e014848bca6f479340e77
    git cherry-pick -x a06d1b5f5d445167b52d07d468a6fde4b600a275
    

    For 10.4.x:

    git cherry-pick -x 71fa6b7c8a0de2168614a7eb83c3a4250ea524c6
    git cherry-pick -x 2fda6b696b1decfc101f9159abdebe91a2384c34
    git cherry-pick -x 0c64906deea9b2064439511548c1c08050707fa4
    
    
    🇺🇸United States xjm

    These two issues probably need to be included in the MRs to get 11.1.x and 10.4.x to pass:

    🇺🇸United States xjm

    @tobiasb This was deliberate. 8.2.0/8.3.0 was overly specific and done probably by reflex, rather than for any valid reason. We should only specify a patch version if we know we require or recommend a specific patch version

    🇺🇸United States xjm

    Added to the 11.3.0, 11.3.0-beta1, 10.6.0, and 10.6.0-beta1 release notes drafts.

    🇺🇸United States xjm

    Added credit for @longwave for documenting the standard practice with our previous issues.

    🇺🇸United States xjm

    I removed core/modules/mailer from 11.3.x today (leaving in place its traits in core/lib, and leaving it in 11.x of course for ongoing alpha development. The other experimental modules are all either already beta or in unique statuses where they are alpha but being kept in tagged releases for special reasons.

    Production build 0.71.5 2024