🇦🇺Australia @acbramley

Made a small change to fix phpstan and avoid adding to the baseline.

This is looking really good and will definitely help testing many other things.

acbramley → made their first commit to this issue’s fork.

Testing for this may be easier once 📌 FIx TODO in Drupal\entity_test\Plugin\Field\FieldType\ChangedTestItem Active is in.

Also needs a reroll onto an MR.

I've tested this manually using Claro and am able to add suggestions to input elements successfully. I also found an example in contrib that also works https://git.drupalcode.org/project/gin_lb/-/blob/1.0.x/src/HookHandler/T...

There may be a missing cache context that is stopping the theme suggestions working across different views in your case.

Are you able to provide any more detail on how to reproduce this from a fresh drupal installation?

Closing as per #22 and #23 - if you are able to reproduce this issue starting from Install Drupal core on 11.x feel free to reopen this issue with the steps you took.

Closing out for now as per #13 and #19

If you are able to provide the required details, feel free to reopen this issue.

Trying to test this out, we need to override the max-age = 0 from EntityFormDisplay, but even with that I'm not seeing cached forms locally.

The IS points to this commit as the cause https://git.drupalcode.org/project/drupal/-/commit/ad6d4462ba6fc9335a59f... but the code changes in the MR are in a different part of the code.

IIUC the commit changed the outcome of calling ->latestRevision() but this MR is changing from latestRevision to allRevisions with a range. While this may fix the performance issue, isn't that just masking other performance issues using latestRevision?

I think this would have to happen after the media is validated, just before the media is saved. We'll also need this in an MR and tests would be great.

let's get 8.x-1.x fixed too

Thanks!

Thanks!

Just going with commenting out those assertions for now, this is blocking other work on the module.

This popped up in triage today, given the age of the issue and that we don't have any steps to reproduce this issue I'm going to close it out now.

If you are able to reproduce this bug starting from a fresh installation feel free to re-open it and provide the steps. Thanks!

We'll need steps to reproduce here to find the bug if it still exists and work out a fix.

@carolpettirossi there's a note in the issue summary about that under Known issues. I would not recommend continuing to run a patch that contains that property as you'll need to run it foreverr.

Funnily enough fixing this error starts throwing more:

Warning: Undefined array key "status" in Drupal\views\Plugin\views\filter\FilterPluginBase->acceptExposedInput() (line 1619 of core/modules/views/src/Plugin/views/filter/FilterPluginBase.php).
and
Warning: Undefined array key "status" in Drupal\views\Plugin\views\filter\FilterPluginBase->convertExposedInput() (line 1505 of core/modules/views/src/Plugin/views/filter/FilterPluginBase.php).

Figured out how to reproduce this one, updating IS.

Removing tag because there's no code to write tests for (yet) and un-assigning since it's been 3 years since.

Is this still an issue in Drupal 11?

I don't think this is something that can be fixed generically, we'd need to fix each instance of these separately.

For example, the IS talks about the Promotion options on the node form. This is 2 boolean base fields, rendered as checkboxes via their widgets and then wrapped in a details element via NodeForm::form

Are there other specific examples we can target?

This field is now readonly with CKEditor5, therefore I don't think there is anything to fix here? None of this javascript exists anymore.

Needs a reroll into an MR

This seems important but has gone stale, I've rolled #28 into an MR and tidied a few things up (attributes, param types, class names, etc) but this still needs work based on #34 and I'm guessing the pipelines will throw a few errors based on things I've missed.

acbramley → made their first commit to this issue’s fork.

This was fixed in #2779321: Submitting empty block layout form results in breakage for all block entities →

Triaged as part of bug smash.

Is this bug still applicable? If so can we get an update to the issue summary using the standard template including steps to reproduce?

I tend to agree with #3 here, given there's a contrib module for this is this something we need to consider for core? I don't think this is a major bug either

Triaged as part of bug smash.

This needs an IS update with the standard template and some steps to reproduce.

Testing this on HEAD and the error message is much more helpful in 2025:

Symfony\Component\HttpKernel\Exception\ControllerDoesNotReturnResponseException: The controller must return a "Symfony\Component\HttpFoundation\Response" object but it returned a string ("string"). in () (line 98 of core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php). 

This comes from symfony's HttpKernel.

IMO this is even more helpful than the error message in the patch ("The result of a controller cannot be a string.") because it gives instructions on what it should be.

With that in mind, maybe we can just close this out now?

This was triaged as part of bug smash.

This javascript no longer exists in core, it was refactored (after being converted from es6) in ✨ Create header component for Umami, with JS Fixed .

The new header JS looks safe, also confirmed by installing Umami and disabling the main nav block and didn't get a javascript error

Should this be closed as a duplicate as per #8?

This was triaged as part of bug smash.

I attempted to reproduce the warning using standard profile with the following steps:
1. Install standard profile
2. Edit the content view and add a block display
3. Add the block to the content region of Olivero
4. Export config using drush cex -y
5. Copy block.block.olivero_views_block__content_block_1 and views.view.content to a tmp directory
6. Install standard profile again
7. Export config again
8. Copy config from step 5 back into the sync directory, retaining the new UUIDs
9. Run drush cim -y

At step 9 we have a create for the block and an update for the view, no warning is thrown.

Setting to PMNMI to see if there are other steps to trigger this issue, otherwise I think we can assume this has been fixed and can close this issue.

::processDate was removed entirely in https://git.drupalcode.org/project/drupal/-/commit/2fc40e8642c95abff47e8...

closing this as outdated as it doesn't look like this error exists in this class anymore, the other pre render callback in the element class sets $element['#attributes']['type'] = 'date'; if it's not set.

This came up as part of the daily bug smash triage.

Is this still reproducible on the latest version of Drupal core? If so, can we update the IS with the steps?

Closing this out for now as it's been over 3 months and we don't have clear steps to reproduce this issue.

Please feel free to re-open this issue with steps to reproduce starting with installing Drupal on the latest stable version. Also please include more details about the exact commerce modules/setup.

The other issue is in, so this isn't postponed anymore.

Got things working and tested manually with non-base fields and it seems to all work nicely. I'm not seeing any test coverage for the filter using a taxonomy scheme on non-user entity views so I'll try to whack together a test.

acbramley → created an issue.

Responding here to the DM from @camilo.escobar

I was added as a maintainer to this project for D11 compatibility, it is currently minimally maintained with most maintainers being quite busy on other work.

I don't see a reason that this needs to be a submodule, you could just have this as a separate project and maintain it yourself. I tend to prefer separate projects from a maintainability point of view.

Setting to PMNMI for now, will close out unless there's a good reason that this needs to be a sub module.

FYI this change means sites without a key or secret set (e.g for local or CI environments) you'll get a PHP fatal error

TypeError: Drupal\s3client\S3ClientFactory::createClient(): Argument #2 ($secret) or TypeError: Drupal\s3client\S3ClientFactory::createClient(): Argument #1 ($key)

Thanks @catch! My vote is closing this as a won't fix.

Thanks!

Back to RTBC

I still want a review of the approach before creating a CR

@oily the fix has been committed to 11.x

@timmy_cos it will just be the label of the node (see EntityViewController::buildTitle)

Is there anything we're waiting on here?

It has been 5 months since #82, wanted to give this one last bump before closing it.

Closing this for now, if you feel strongly that this should be implemented please feel free to reopen.

Now expanded to the generic revision UI as well.

Same failure as #41 with test-only changes "0 elements matching css "table tbody tr" found on the page, but should be 50."

Added test coverage for the issue, here's what the revisions page looks like on HEAD in that test case:

revisiting this with fresh eyes I realised we needed to filter by revision_translation_affected, adding that means we are back to where we were in terms of revision displays (tests pass with no changes). Now we need test coverage for what we're fixing (e.g broken pagers with a lot of unaffected revisions)

Took me a while to figure out what was going on, the DataProvider was pointing to the wrong method!

@mrshowerman thanks very much for the link, confirming that the fix from that issue (https://git.drupalcode.org/project/drupal/-/merge_requests/12124) does fix this issue. Agree that it should be closed as a dupe.

Confirming this fixes the issue for me, I found this after upgrading to 11.2 and Gin 5. Back to NR.

acbramley → made their first commit to this issue’s fork.

It would be possible to leave the library empty

I tried that and you can't, you get an error about an incomplete library

I've tested across Gin, Claro, and Demo Umami (with editing in admin/fe theme) and these classes aren't used in any of them. I think we can even remove the node-edit-form.html.twig from umami as well as that is not used at all.

To make things even stranger:

node.module.css is in 2 different libraries - node/drupal.node and node/form
Both libraries are only attached in a single place in core - NodeForm.php
Claro removes the node.module.css file from both libraries via libraries-override
stable9 replaces node.module.css from both libraries with its own node.module.css file

I think in this issue we should also remove the node/form library entirely.

Looking at node.module.css we have styles for:

1. .layout-region this only has one rule box-sizing: border-box; which Claro also has in its form-two-columns.css. This looks pretty safe to remove
2. .layout-region-node-main and .layout-region-node-footer and .layout-region-node-secondary These classes only appear in 3 places that aren't CSS files. Those are node-edit-form.html.twig in the demo_umami theme, the starterkit_theme, and in gin itself. Gin also has a lot of CSS rules for these same classes.

Funnily enough, none of these classes (apart from layout-region) are on the node form after installing standard profile both with and without the "Use the administration theme when editing or creating content" setting.

TLDR; these are looking quite safe to remove. I will need to test across a few of these areas as well as with the vertical toolbar enabled as there are some special rules for that too.

Also need an IS update here, I'm guessing from the existing patches this is just referring to the styles in node.module.css

Here's the duplicate 🐛 Revisions are not visible for some nodes Needs review

This is a duplicate of 🐛 Node revision UI will sometimes show no revisions on a page RTBC and 🐛 Revisions are not visible for some nodes Needs review

They should actually be removed, postponing this on 📌 Consider showing all revisions on the revision overview Active although I think we have other duplicate issues in the queue.

That does reflect the database but will it make sense to the editor?

I agree it's quite a large change, but if you look at all the related issues and #27 you'll see the current situation is really not ideal. I'd rather see more detail than not be able to see what's actually going on.

I think the reason why we see new revisions for the default language is because revisions are created for the default language as well?

Thanks for working on this, it should be straightforward to add test coverage for this to the existing tests

Nightwatch tests are extremely inefficient and are likely being phased out eventually (see 📌 Consider dropping Nightwatch in favor of Functional Javascript tests Active ).

The test here can be rewritten as a functional test that just checks for the label and uses a test view that's already defined in yaml.

We found issues with this hunk of code after deploying to production:

    if (\Drupal::moduleHandler()->moduleExists('path')) {
      $route = \Drupal::service('path.validator')->getUrlIfValid($source_path);
      $allow_alias = $this->config->get('allow_from_alias') ?? 0;
      $skip_alias = !$allow_alias;
      if ($route && $skip_alias) {
        // This URL path has a valid internal route and the Redirect settings
        // are not configured to allow redirects from aliases. Do not redirect.
        // See https://www.drupal.org/project/redirect/issues/2879648
        return [];
      }
    }

This was breaking preview_link functionality through some weird interactions with getUrlIfValid.

This was added in https://git.drupalcode.org/project/redirect/-/merge_requests/109/diffs?c...

@mark_fullmer do you recall exactly what this was achieving? From my testing, we don't need this check at all. With some better handling in the path processor we can simply not check for the alias when the config isn't set (see latest commit).

I also fixed an assertion in one of the new tests.

I've reverted the documentation page.

The tricky part here is that setComponent is used both for adding fields to the layout when Layout builder is initially enabled, and when adding new fields.

Both in the case of the old feature flag and removing the code outright, we get an empty layout when enabling layout builder. IMO this is a good thing, but just wanted to point that out if it wasn't obvious before.

We could retain that functionality and just move the code out of setComponent and into a new method called by the preSave which is where the initial addition of the fields when layout builder is enabled happens.

acbramley → changed the visibility of the branch 3115759-prevent-auto-adding-new to hidden.

acbramley → changed the visibility of the branch 3115759-prevent-auto-adding-new to active.

What's your timeline on getting 2.x stable? Why don't we do a 3.x branch that is 8.x-1.x that drops unsupported versions of core/php. Once 2.x is stable that could either continue on a 2.x line or become 4.x.

I understand where you're coming from, but even Drupal 10 requires at least PHP 8.1 → . Postponing 8.4 fixes on the off chance someone is using an ancient version of PHP feels backwards to me, especially since we're soft-blocked on a new major given the architectural changes in 2.x that is not production ready.

acbramley → changed the visibility of the branch 3496146-deprecations-php-8.4 to hidden.

Green!

Might be good to get 🐛 Resolve test failures from once() called statically Active in first to get a green pipeline for this.