#Needs tests

This test is bad as written because upload_max_filesize and post_max_size can't be set with ini_set().

dcam → changed the visibility of the branch 11.x to hidden.

dcam → changed the visibility of the branch 2557319-dont-display-size to hidden.

Been having this issue.

I can recreate by creating a queue, adding all nodes, and rendering them. Most of my content types use a taxonomy vocab and probably like 90% use the default term. That default term also has a media attached to it.

Code isn't too bad,

- a large amount of nodes
- load them,
- then,

        $view_builder = \Drupal::entityTypeManager()->getViewBuilder('node');
        $render_array = $view_builder->view($node, 'default');
        $output = \Drupal::service('renderer')->renderInIsolation($render_array);

After like 25 nodes, it starts throwing errors. After banging my head, found https://www.drupal.org/project/search_api/issues/2913931 → which lead me to this issue.

@jonathanshaw in #183 has the best, most succinct description,

> we don't have a recursive limit, we have a repeated rendering limit.

Languages is one thing, but definitely not the only thing to consider. We need a better mechanism to map the references.

I rebased the MR.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I converted the patch in #2 to an MR and added the schema verification tests. Some of the original changes are irrelevant now because this issue is so old. But the CKEditor plugin schema is of particular note. I noticed there's no fallback for ckeditor5.plugin.*, but when I attempted to add one other tests started failing with the following message:

Failed asserting that exception of type "TypeError" matches expected exception "Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException". Message was: "Cannot assign Drupal\Core\Config\Schema\Ignore to property Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::$schema of type Drupal\Core\TypedData\TraversableTypedDataInterface"

So there's an check for a specific data type here. I'm not sure if this invalidates the need for a fallback schema or not. I ended up removing that change from the MR.

Also, I don't know if there are other new plugin types in Core that need this treatment. If anyone knows of some, then let me know.

I agree that this is valuable beyond just core. Today I need it for drush site:install - https://github.com/drush-ops/drush/issues/6310.

Reroll of #73 for Drupal 10.5

Great work @f.mazeikis - did another review and added some comments/observations

I just hid a branch because the ...11.x branch is the active one..

oily → changed the visibility of the branch 3326684-deprecated-function-mbstrtolower to hidden.

Since there's been no follow up going to close this one out. If still a bug in D11 please re-open

Also attaching updated patch

I created a new issue fork and MR for D11: https://git.drupalcode.org/project/drupal/-/merge_requests/12682

I collected the existing patches and merged them in this issue fork.

I uploaded the patch for your composer-patch as 3326684-42.patch

But this is only the mitigation. As @geek-merlin wrote in #37: behind every warning waits a bug that should have been fixed before.

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

So we have a new MR now that is against 11.x branch. Tests are still missing.

yannickoo → changed the visibility of the branch 11.x to hidden.

#175 applies to 11.2 either. Thank you.

Patch #64 worked fine for me. Running 10.5.x version of core.

Addressed everything on MR and in #11.

The MR conflicts were resolved.

No longer applies

The tests seem to be working as expected. +1 RTBC

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

We still need more information, I agree with #1 that the comments are a bit superfluous as they're just stating what happens when you save a node with a langcode. Unless we want to heavily document what a fallback is in the test (I don't think we should) maybe we should just remove the entire comment, or just make it "Creating a public node with langcode Hungarian"

We should also update the IS with the exact comments we want to change (remove the original report) and update the proposed resolution.

MR isn’t pointed to the right branch. And appears to be missing test coverage

I am happy that I found this and never needed to use that in my time working with Drupal - until now :D Even when having the patch applied I cannot see a differen action attribute in the exposed filters forms.

Do you think we should also try to read that configured Custom URL first before taking the <current> URL like this?

$form_action = $view->display_handler->getPath() ?: Url::fromRoute('<current>')->toString();

Can the issue summary be updated to use a standard template please :)

Also was previously tagged for tests still needed I'm assuming?

Was there a merge conflict needed for the rebase?

Feedback has been addressed it seems.

Saving credit for alexpott for the MR reviews.

Added 101 commits from source branch. Re-ran failed functional test. Pipeline is now green.

The PHP intl extension is compiled into PHP in the docker image. If the PHP intl extension should be optional, the base docker image has to be changed to install intl with pecl.

I compared the composer.json ext-* requirements and the extensions installed in the gitlab docker image. There are many PHP extensions installed that are not listed as required by core and its dependencies. Doesn't this lead to potential post-installation problems on systems that don't have the usual large number of PHP extensions installed? If the PHP extensions are only suggested,, they should be disableable for testing.

I think comments have been addressed.

Awesome! Thank you.

Thanks for adding test coverage and the fixes here.

I think that's all that can be tested here, thanks for working on this.

Committed 0e7d63c and pushed to 11.x. Thanks!

longwave → closed merge request !12165

Thanks @joao.ramos.costa, adding the Drupal core AJAX issue you mentioned as related issue, since fixing that might take care of many other issues in contrib modules.

This is intended to cause a fatal error, that is what is being tested that Upgrade Status does not collapse on scanning a module with a PHP fatal error in it :) So not sure what you mean here? If we would comment out that line it would not cause the fatal error that we need.

How can we proceed with this?

Triaged as part of BSI and reproduced this bug manually, still an issue on 11.x. Some of the reported issues in the IS aren't reproducible though.

Rewriting the IS with the standard template.

Postponed on ✨ Provide a Entity Handler for user cancelation Needs work as per #92

Thanks for adding tests here.

heddn → closed merge request !27

heddn → closed merge request !26

heddn → closed merge request !1

Seems a few other block--system-menu-block.html.twig in core. Is there a place we can add an assertion to?

Agreed that modifying the Twig templates is not the best approach here. Instead, we can supply the ID attribute to all templates through the SystemMenuBlock::build() method. The rendering logic for a unique ID can be modeled exactly on block/src/Hook/BlockHooks::preprocessBlock(), per below:

    // Create a valid HTML ID and make sure it is unique.
    if (!empty($variables['elements']['#id'])) {
      $variables['attributes']['id'] = Html::getUniqueId('block-' . $variables['elements']['#id']);
    }

I've created an alternate merge request, at https://git.drupalcode.org/project/drupal/-/merge_requests/12664

My previous comment was wrong. This is still an issue in Drupal core, though it only manifests in the context of placing a menu block using Layout Builder.

heddn → closed merge request !1

Lets try that! (Sorry for the delay so many issues now!)

Feedback appears to be addressed on this one.

This appears no longer to be an issue as of 11.3.x. Placing a menu block on the page successfully generates unique ID attributes; placing a second instance of the same menu block confirms that the ID attributes and corresponding aria-labelledby attributes are unique. I haven't tracked down the core commit responsible for this yet, but will set this to "Postponed" given the above.

Example markup of first instance of menu block:

<nav id="block-olivero-mainnavigation" class="contextual-region primary-nav block block-menu navigation menu--main" aria-labelledby="block-olivero-mainnavigation-menu" role="navigation">
      
  <h2 class="block__title" id="block-olivero-mainnavigation-menu">Main navigation</h2>

Example markup of second instance of menu block:

<nav id="block-olivero-mainnavigation-2" class="contextual-region primary-nav block block-menu navigation menu--main" aria-labelledby="block-olivero-mainnavigation-2-menu" role="navigation">
      
  <h2 class="block__title" id="block-olivero-mainnavigation-2-menu">Main navigation</h2>

Looking promising!

1. This needs to have preliminary internal HTTP API support. "preliminary", because it might be imperfect, and that's fine. But we need to have the FE folks unblocked to build the entire feature; we can then still refine it later 👍
2. That MUST come with updated test coverage in \Drupal\Tests\experience_builder\Functional\XbConfigEntityHttpApiTest::testJavaScriptComponent()
3. Some missing test coverage to ensure that saved config entities in config entity storage do not actually contain these base64 blobs

Created a quick and dirty patch for 2.2.x (installed dev-2.2.x c97badf) based on the changes in https://git.drupalcode.org/project/layout_paragraphs/-/merge_requests/11...

Seems to work for me.

I experience this exact issue: if the paragraphs on the secondary language are edited in the front end editor (Experimental Layout Paragraphs Builder), all paragraphs on the original language are gone and overridden by the paragraphs on the secondary language.

I'm using 2.1, and seen this issue in 2.2. Is there any fix for these versions?

I think that's long enough :)

longwave → closed merge request !12273

!12273 is green now so this should mean no test failures once this is committed to 10.5.x and 10.6.x. Thanks for the patience and assistance with the backports here.

Committed and pushed 86faa69f084 to 10.6.x and 53f117df5f0 to 10.5.x. Thanks!

longwave → closed merge request !12272

longwave → closed merge request !10452

So currently this is in all supported 11.x versions but not 10.x.

✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review landed in 10.6.x and 10.5.x finally, I've rebased MR !12273 against 10.5.x and if this is green now then this means this can be committed to both those branches.

I'm also closing the 11.0.x and 10.4.x MRs as they are no longer eligible for backports.

@martins.bruvelis would you mind testing 🐛 Field formatter with inline settings is missing field formatters from the referenced (media) entity type Active and see if it fixes these issues for you and works correctly? Thanks!

Added the test cases,
Please review!

Working on it!!

Thanks for writing tests in this space.

Re polyfill:
Regardless of core version no reason XB couldn't add it as a dep, just give it a loose constraint so it doesn't conflict with core.

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

Some comments posted below on the code. As far as tests, we are constantly attempting to increase test coverage. Functional tests can mock guzzle responses or work with real test endpoints. Meaning, you could create a test controller/form in a test module that returns all the expected outputs.

1. +++ b/src/Plugin/migrate_plus/authentication/Cookie.php
   @@ -0,0 +1,106 @@
   + * Provides cookie authentication for the HTTP resource to another Drupal 8 site.
   

   Let's not put versions in comments. Makes maintenance easier if we don't have to keep abreast of versions in the code.

2. +++ b/src/Plugin/migrate_plus/authentication/Cookie.php
   @@ -0,0 +1,106 @@
   +      'base_url' => $this->configuration['domain'],
   

   if we make this a "url" config option, then we can divine the domain name from it.

3. +++ b/src/Plugin/migrate_plus/authentication/Cookie.php
   @@ -0,0 +1,106 @@
   +    $loginClient->post($this->configuration['domain'] . '/user/login', [
   

   can we make this more drupal agnostic and have the url be to the login URL?

4. +++ b/src/Plugin/migrate_plus/authentication/Cookie.php
   @@ -0,0 +1,106 @@
   +    if ($this->configuration['rest']) {
   

   this is very tied to drupal paths. more generic is better as it become more usable across different scenarios.

Thanks everyone for your contributions on this issue.

We can work around this in beta by uploading images to media library and manually referencing them in component code. This is obviously not great and doesn't allow components to be moved from environment to another so we definitely still need this. Just trying to avoid rushing this before the beta.

Re-roll patch #17 for 11.2.x

Can we add tests that show this broken and it fixed?

I've tried the patch, but as with my hack, I get my homepage showing as http://mysite.com/mypathalias rather than http://mysite.com/.

#6: That should be fine, because XB bumped its requirement to 11.2 in 🐛 PHPUnit Next Major tests failing Active ? That's what's in 0.5.0-alpha1 → , so <0.5 is for Drupal <11.2, >=0.5 is for Drupal >=11.2

That was pretty fast! Reviewing… 🤓

Rerolled the patch for Drupal 10.4.6.

> I instead just got the normal view, listing all content, completely ignoring that the “Title” filter should have been required.

I think we need to define what is the expected behaviour when you load the page without a filter value by loading the URL rather than by submitting the form with the button. The problem is though, is it possible to tell the difference in PHP, since it's a GET form?

> When trying to submit the form without filling out “Title”, the browser (Mozilla Firefox 122.0) prevented me from doing that, but after manually removing the required attribute from the DOM this again worked as if the “Title” filter wasn’t required

I don't think this is a problem -- if someone edits their DOM then they're on their own.

Pushed all the work so far, including working tests. @larowlan had some feedback, I think I've addressed most of it, but happy to get back to it if there's more.

Moving into ready for review.

Created MR, please review it.
Thanks!

Don't see anything to review?