#Needs tests

Automatically closed - issue fixed for 2 weeks with no activity.

This came up as daily BSI target.

Since it's kinda already been triaged I'm going to move to PMNMI because no one else seems to be reporting it since 2017 is this still an issue in D11?

wonder if this should be closed or re-opened?

Since there's been no follow up to #19 or movement really since 2017. Going to close out but if still an issue in D11 please re-open

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

@larowlan mentioned he wasn't happy with one area. I investigated and implemented a counterproposal.

There is a report that this regressed.

AFAICT this also blocks #3519878 now, see #3519878-22: [PP-1] ContentCreatorVisibleXbConfigEntityAccessControlHandler's `view` access must refuse access to disabled config entities → . IOW: I think/hope that once this is fixed, the remaining test failures there will disappear.

The test coverage here is definitely sufficient, so removing tag.

P.S.: This removed a few @todos pointing to 🐛 SdcPropKeysConstraintValidator::validate() should complain about extraneous keys too, not just missing keys Active , because those bits were getting in the way of getting this MR to pass. This is critical for data model stability, so this takes precedence.

See comment #59 🐛 Allow views attachment display to use its own pager options Needs work , posted two years ago.

Thanks! Will review 😊

This deprecation is now triggering fails in another MR that adds a views upgrade path https://git.drupalcode.org/issue/drupal-3095893/-/jobs/5480383

Got something going that uses the schema, but I'm not 100% happy with it.
I think its probably ok, but there's a chicken-egg scenario with creating a version string vs having a component typed data adapter that prevents using being able to use \Drupal\Core\Config\Schema\TypeResolver::resolveDynamicTypeName

The workaround seems ok, but is not ideal.

I would like to unblock 🐛 Remove duplicate "add block" link from block content type view's "Results not found" message Postponed which was postponed based on #121.4

IMO having duplicate links on a page that go to the same place isn't necessary. The action buttons are a much more prominent pattern in core and things like Node and Media don't and haven't had an Add link in the views table so people will be used to using the Add content/media buttons.

If we still want the duplicate link in the list controller table that this issue is adding I think that's fine, but adding a generic area plugin for views seems like a much larger task and should either be agreed as a won't fix or moved to a separate issue?

🥳

BTW, 📌 Constraint slot names allowed by XB in its component tree Active added a todo pointing here — this MR should be able to drop those lines.

Re #12 that makes sense.
I think we can include the 'current schema' in the hash calculation and that gives us item 3 from the issue summary

Will pick that up today

Refactored from a functional test to a kernel test.

#6: gave this some more thought.

You’re right that it’s impossible to validate old versions (any besides the active one) if they depend on data not contained within the config entity.

So … I think the solution is actually very simple, plus it would prevent brittleness in the future: only validate active_version!

That way, even if a ComponentSource plugin changes what information is used to deterministically compute the hash, old versions (hashes) continue to work fine. Because in the proposal I made in the issue summary, any such logic change would have caused all old versions (hashes) to become invalid 🙈

And that is actually perfectly in line with the source-specific settings for old (non-active) versions: those already use type: ignore because the config schema for older versions might have been different. What we’re dealing with here is similar.

Updating #174 (2985168-175.patch) to include a hunk from #172 that seems to have been missed or accidentally removed (2 hooks in media_library.module).

The re-added hunk:

• Hides the "Delete" button from the modal
• Makes the modal "Save" button submit via AJAX
• Gets the media-item in the node edit form updated with any changes saved using the modal

Minimally tested, it works well with Drupal core 11.1.7 + Gin theme.

Didn't get to reviewing this today :/

Hi,

Just updated a site to Drupal 10.4.7 and Metatag 2.1.1 and using the diff from the MR the Metatags are configurable per field instance.

Attached in the patch file to use in composer patch json (we don't link directly to the drupalcode diff file for security and stability reasons) based on the commits upto d6d6d892 .

Note that for anyone using the earlier patches then you need to reconfigure each field due to the change from the select list to the checkboxes but I think checkboxes are definitely more user friendly.

Also, I was thinking there must have been a bug somewhere as I was seeing the following metatag fields 'Max Snippet', 'Max Video Preview', 'Max Image Preview', and 'Unavailable after date' but no way to manage them. But diving into the code I found they were part of the Robots plugin. Not sure if there is a UI way to make that a little more obvious or whether it would make the admin page too cluttered. This isn't really an issue with this code more a general one - only it is more obvious with this code as you cannot turn those sub fields off.

Cheers, Neil

Just a diff patch against --branch 10.4.x to address this and other warnings, prior to new release

#5: block plugins’ default configuration might stay the same, but the validation constraints might become tighter or looser, which would then affect the freedom of component instances. Which then could lead to existing instances failing to render, and/or the need for an update path.

#7: yep, this straddles both. But component source plugins just must work well enough for beta1, the API is not expected to be final by beta1. Data storage must not change after beta1, and version hashes changing would be … bad.

Will review, curious what you came up with!

I might have a way to do it with raw config typed data

I think if we inject the source manager into the constraint validator we could boot an instance without the config entity, I think that's acceptable, will do that.

This puts us into the Production ready component source plugins meta as much as the data storage one as it might require new methods on the interface

I think this raises an issue if we're generating the hash based on information that isn't stored in the config entity.

Specifically things that aren't available in \Drupal\experience_builder\Entity\Component::validateVersions in order to validate the configuration.

It would require making that rely on actual source plugin instances and consulting them to ask what else should be considered in calculating the expected hash.

Will have a think about how to approach that, I don't know that booting an instance of the source plugin during validation is the correct approach when we're dealing with typed data (specifically config mapping data) and not config entities.

the combination of a normalized config schema for that block plugin, plus the block plugin's ::defaultConfiguration

From BlockManager

$settings = [
        // We are using strict config schema validation, so we need to provide
        // valid default settings for each block.
        'default_settings' => [
            // The generic block plugin settings: all block plugins have at least
            // this.
            // @see `type: block_settings`
            // @see `type: block.settings.*`
            // @todo Simplify when core simplifies `type: block_settings` in
            //   https://www.drupal.org/i/3426278
          'id' => $id,
          'label' => (string) $definition['admin_label'],
          // @todo Change this to FALSE once https://drupal.org/i/2544708 is
          //   fixed.
          'label_display' => '0',
          'provider' => $definition['provider'],
        ] + $block->defaultConfiguration(),// 👈️👈️👈️👈️👈️
      ];

We already include default configuration - does that cover off the schema already?

Can you provide an example scenario where we need item 3?

Will make a start on item 2.

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Can hardly be a bug if #3528159 has just landed.

Automatically closed - issue fixed for 2 weeks with no activity.

Repurposing the issue to remove all comments logic as we no longer use the feature and we don't need to maintain the code around it.

andypost → closed merge request !2210

Automatically closed - issue fixed for 2 weeks with no activity.

I think this is good background reading for the other issue, but let's explicitly postpone it on that one.

Yes, extending existing Render Elements classes (as proposed in 📌 Slowly, very slowly start OOPifying the render system Needs review ) looks better than adding a Builder class for each Render Element as proposed in the current MR.

Also, when we will replace most (but not all 😉) of Render & Form elements by SDC, all the logic to remove will be contained in a single file.

Patch #15 is totally ok to convert to MR and add tests, it brings visibility at least

the same time core (field ui) still has no UI to even know that there's any field's table and definitions exists...
Moreover body field for node/comment is created automagically

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Just leaving a note, in case someone else also who simply want to translate a string finds this issue. Because without the patch, in a view which has results, in order to get Twig to render a string in a "Global: Text area" in the "Header" section in Drupal 10, I need to enable "Use replacement tokens from the first row" and only then is a string such as {{ ('My string.'|trans) }} translated.

I spent quite some time working on making just 1 conversion, I agree with #153, this is not the way to go about it. 📌 Slowly, very slowly start OOPifying the render system Needs review is a much nicer solution.

I tried #142 but the amount of methods you'd need to add is staggering and at the end of the day since people use arbitrary properties in render arrays you would need to support some sort of set/get anyways so I leaned heavily on that and so 📌 Slowly, very slowly start OOPifying the render system Needs review is born. If that issue gets accepted this issue might be outdated.

Removed the @todo about code duplication since it was investigated in 37.

Also, since @larowlan has looked at the MR, and he's a Form API maintainer now, I think this is ready.

#71 worked correctly for us https://www.drupal.org/project/drupal/issues/2833734#comment-15844466 🐛 Allow views attachment display to use its own pager options Needs work

Had a look through tests and I think this will be very hard to cover without being flakey.

AFAICT it needs to be in a JS test to do the draggable stuff (reparenting isn't supported on the menu form without javascript). We have a functional test in MenuLinkReorderTest which does almost nothing, and the existing JS test coverage only tests contextual links.

Reproduced this and the patch from #50 fixed it nicely. Rolling into an MR, still needs tests.

Tested this using Standard profile and initially I was confused because it seemed as though I could reproduce it.

However, my issue was that I simply didn't click the green tick after toggling on "Decorative image" to save the changes. After doing that the image correctly gets an empty alt attribute. There's even a warning over the image if you don't do this and leave the alt text field blank.

Should we close this in favour of ✨ Local tasks for node not visible on diff.revisions_diff route RTBC ? Local tasks seem like a more appropriate solution.

I'm still encountering the following error on the 11.x branch:

LogicException: The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution. in Drupal\Core\Database\Connection->__sleep() (line 1309 of /var/www/html/core/lib/Drupal/Core/Database/Connection.php).

I've also re-rolled the patch from #15 to apply cleanly against the 11.x branch.

✅ Patch tested and verified on Drupal 10.4.7

This patch applies cleanly and resolves the issue where an attachment display in a View cannot render pager values when inheriting from the parent display.

🔍 Steps to reproduce and test:

Created a View with a paged display (e.g., page display with a pager).

Added an attachment display that references the paged display.

Enabled the Render pager option in the attachment display.

🔧 Result:

With the patch applied, the attachment correctly renders pager values when configured to inherit and render them.

Without the patch, the attachment display does not show pager output, even if render pager is enabled.

🧪 No regressions observed. Views behavior remains stable, and the patch only touches Attachment.php.

📦 Patch is based on an updated fork where the original patch was implemented but not applying cleanly. Reformatted to Drupal standards using a/ and b/ paths.

✅ Marking as Needs Review — patch applies cleanly, fixes the issue, and is safe to test.

Following up if anyone can update IS with steps, if no follow up could close out in 3 months.

Since there's been no follow up going to close out, but don't worry! Can always be re-opened.

I resolved the phpstan issue by adding " // @phpstan-ignore argument.type" comment by taking reference from

KeyForEverySdcProp

validator file. Please review and let me know if anymore changes are required.

Unassinging myself so that someone else can work for test cases and keeping the issue to needs work only.

Although, when using a custom field formatter for file fields it's not clear how to set this option within the field formatter. The link URL object is generated in the preprocess function, and there's no way to pass additional attributes to the link element, only the warapper element.

Confirming #60 by setting the target for the link to _blank, which was my main use for this. I can't comment on anyone else's requirements.

function MY_THEME_preprocess_file_link(&$variables): void {
  $attributes = $variables['link']['#url']->getOption('attributes');
  $attributes['target'] = '_blank';
  $variables['link']['#url']->setOption('attributes', $attributes);  
}

I think #24 is a valid concern (see 🌱 [policy] Treat username enumerations as security bugs that require Security Advisories Active ).

prudloff → changed the visibility of the branch 2842858-basic-auth-module to hidden.

prudloff → changed the visibility of the branch 10.4.x to hidden.

prudloff → changed the visibility of the branch 10.3.x to hidden.

Since there's been no follow up, going to assume this has been fixed in the related issue.

Since there's been no follow up in 3 months going to close out. But don't worry! Can always be re-opened

Thanks all!