#Needs tests

I think this looks good, and would be an excellent thing to add to D7 (even this late in the day).

However it has to be an opt-in change.

Just sanity checking that the code only enforces the host pattern checking if the variable is set:

  // Check trusted HTTP Host headers to protect against header attacks.
  if (PHP_SAPI !== 'cli') {
    $host_patterns = variable_get('trusted_host_patterns', array());
    if (!empty($host_patterns)) {
      if (!drupal_check_trusted_hosts($_SERVER['HTTP_HOST'], $host_patterns)) {
        header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request');
...snip...

So if a site doesn't define any patterns, there's no change.

Site's can add patterns (e.g. in settings.php) and they will then start to be enforced.

If that's all true, +1 from me; I'll double check this and then am happy to commit.

Not everyone is on 11.x yet... This is rerolled for 10.3.x.

I've added the patch #169 and i have this error :

TypeError: array_diff_assoc(): Argument #2 must be of type array, null given in array_diff_assoc() (line 154 of modules/contrib/redirect/src/RedirectRepository.php).

$query does not exist in findRedirectByHashes function.

I have committed the changes to fix the MR, now the failing tests are fixed. Please check

Path #7 added the schema, only extending the search_api.index.* schema did not work correctly

Updated the config schema to work with third_party_settings with their own config

Removed the tag and added a screenshot, but the issue still persists.

I have checked MR #15 in Drupal 11.
It fixed the Olivero issue on mobile, but the issue still persists in Claro.

Olivero
Before MR

After MR

Claro
Before MR

After MR

Discussed with @catch and he agreed it made sense to backport ✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review

Found this issue while looking 🐛 Referencing new media requires "administer media" permission Needs review which is dealing with the same issue - only for the view own unpublished media permission / MediaSelection handler. I set that issue to postponed as I believe the backwards compatibility handling and UX review happening here are needed and applying the solution from this to the media selection could be done as a follow up to this issue.

Looking at the feedback from the accessibility review

> At the moment you are unable to distinguish between published and unpublished nodes while scrolling the list of available results. Unpublished nodes should be labeled in the list of options in the autocomplete field in some way.

I would suggest it should be a follow up, since this change is not introducing this problem. Seeing unpublished content is currently the case for users who see unpublished content by way of the existing checks so changing the label could be done independent of this work?

I have rebased the MR against changes latest - looks good to me.

Confirming patch #39 🐛 Endless Redirection loop when "Enforce clean and canonical URLs" is enabled Needs work works good on D10.3.10, PHP8.3 and version 1.10 of the Redirect module.

Just created a patch from the MR so I can apply it in my composer.json.

Thanks all, especially @mxr576 for sticking with this one.

:beers:

A backport to 10.3.x or 11.0.x would also require a backport of ✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review which as a task isn't eligible for backport, so marking this as fixed.

Does this highlight an edge case in the backporting policy? Here’s the dilemma:
This fix cannot be backported to 11.0.x due to its dependency on a non-backportable issue. As a result, the upgrade path from 10.4.x/10.5.x to 11.0.x could lead to unexpected and unforeseeable feature or bugfix losses unless projects skip directly to 11.1.x.

Patch #42 works in Drupal 10.3.10.

Personally, I think this is is quite a crucial feature to have because without it, sites' sites/default/files/inline-images folders will be in danger of becoming dangerously bloated, I know mine is!

@smustgrave I have updated the current MR and closed the other one. Please check

ramprassad → changed the visibility of the branch 1156338-fixed-maximum-number-1 to hidden.

ramprassad → closed merge request !10400

There is a contrib module that provides this service → , should we close this ticket?

larowlan → closed merge request !10409

Backported to 10.5.x and 10.4.x

A backport to 10.3.x or 11.0.x would also require a backport of ✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review which as a task isn't eligible for backport, so marking this as fixed.

Thanks all, especially @mxr576 for sticking with this one.

Loading alias info into global drupalSettings allows it to be available to the ckeditor5 linkit plugin on init so the alias can be upcast into the model. Storing in drupalSettings also allows it to be available to all text formats and persistent between switching text formats.

fabiorubim740@outlook.com → changed the visibility of the branch 10.2.x to hidden.

Please add to an existing MR vs starting a new when the current is pointing to the correct brandh

@smustgrave I have created a new MR (10400) with an update hook in file.install which updates the add_more:false in the form display config for the applicable bundles where these multivalued fields are available. This includes the changes in the previous MR.
My opinion would be to have add_more:true, to have this feature enabled by default to give a good user experience to content authors. Please review and suggest.

I am working on a d7->d10 migration with 220 field_instance(s) to be migrated. It was frustrating and time consuming to get crashes of "drush mim" due to this array_filter issue (yes, crashes). I was not brave enough to suspect the core code to have a problem. It was only after i found this issue thread here that i simply removed the function call (patch #6) and all the problems disappeared! I am so happy, that you discussed this here.
However, i think the patch should find a way into the core code for all the d7+ drupal versions.

10.4.x backport is ready in the related MR.

It seems ✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review has not been backported to 11.0.x yet, so that should be backported first.

📌 Add a method to access the original property Needs work at least partially addresses this, I'm not certain if the current implementation there is up to date, we can still keep this issue to add the test coverage and verify that it does.

I tried to create a new 11.0.x version of this patch, but there is something a little messy in there, perhaps something else did not get a backport, im not too sure. I'm not sharp enough right now to find out.

Re-roll seems good!

Rebase seems fine.

Committed to 11.x and backported to 11.1.x

This doesn't apply to 11.0.x/10.5.x or 10.4.x so needs reroll.

Flagging as patch (to be ported) on that basis.

Thanks everyone for the work here

larowlan → closed merge request !8198

Updating issue credits

Thanks, makes sense.
However, we are now using issue forks and MRs for development, so I created one and applied your patch.
Also, when posting patches an interdiff would have been very helpful (especially when you first rebase and then make further changes).

Still NW for tests and hook documentation.

drunken monkey → closed merge request !5

Thanks for adding the tests @akalata! The tests looks good.

I am just not sure about the added check: !empty($host) (see here). I am not sure about this check - if the host will be empty, are we supposed to return TRUE? When could that happen? Or should we check for this situation sooner in drupal_valid_http_host(), when the condition for CLI was also added?

All tests are green, as mentioned by @avpaderno - https://git.drupalcode.org/issue/drupal-3444017/-/pipelines/298319 .

As the original backport was created by me, adding a tag for the review from another D7 maintainer.

Looks good to me now.

I updated the CR thanks!

Can the maintainer give his opinion on this generic solution for ignoring paths?:

✨ Ignore specific paths Needs work

I think that approach will clash with the one proposed in this issue above.

Also the UI is not very clear:

+      '#title' => $this->t('Apply sub-paths aliases to admin routes.'),

For the user it is not clear what "admin routes" mean. Does this also apply to taxonomy and user edit paths, for example?

I would avoid the word "route" because it is quite technical and because elsewhere in the UI it says "paths".

My proposal would be: "Provide sub-path aliases for admin paths"

+      '#description' => $this->t('This will enable routes like node/[id]/edit to also be altered.'),

This is a bit hard to understand.

My proposal would be "Creates aliases for admin paths like node/[id]/edit and user/[id]/edit"

+      '#title' => $this->t('Apply sub-paths aliases to layout builder route.'),
+      '#description' => $this->t('This will enable routes like node/[id]/layout to also be altered.'),

Same issues.

My proposal would be:
"Provide sub-path aliases for layout builder paths"
"Creates aliases for layout builder paths like node/[id]/layout"

Because tests are failing, I added a task to the list that says "Try to adjust the code or the tests so that they pass.".

Shorter title.
Adding parent issue for reference.

The change was committed to 10.1.x in March 2023. It was not back-ported to D9 but that does not matter now, so I think this can be marked as 'Fixed'

I've added the patch #169 with Drupal 10.3.10 and Redirect 8.x-1.10 ,
Getting below error. Anyone have any solution for this?
Issue : Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ') ORDER BY LENGTH(redirect_source__query) DESC' at line 1: SELECT rid FROM "redirect" WHERE hash IN () ORDER BY LENGTH(redirect_source__query) DESC; Array ( ) in Drupal\redirect\RedirectRepository->findRedirectByHashes() (line 142 of /code/web/modules/contrib/redirect/src/RedirectRepository.php).

This looks good to me, the only question is about BC, I think EntityFieldManagerInterface is a 1:1 interface and we can add methods there, but it could still break of someone for example decorates that service.

Implementing the interface finds finds only one contrib, and that's a false positive, that has its own Interface, it also doesn't extend core.
https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22imple...

extends finds a few, two test stubs and one real subclass, those _should_ be fine:
https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22exten...

Who knows what kind of shenanigans people did with custom code :shrug:

Was about to RTBC, but the change record should have a before/after example even if it's trivial. It possibly should also more clearly say that this method has been added to EntityFieldManagerInterface.

Adding patch for the 10.3.x release cycle since current 11.x patch is not cleanly applied on 10.3.x.

I also noticed this issue while working on the project.
Drupal core: 10.3.10
Issue was noticed on layout builder edit form where core/components.navigation--toolbar-button library was added twice.

Upon debugging, I noticed that it is present twice in ajax_page_state as well.

I think we should apply array_unique when we push the libraries to ajax_page_state so we always have unique libraries in the ajax_page_state.

I've added a PR with the fix.

bootstrap 3.33 release solves the issue, rather than use the core patch I suggest upgrading to bootstrap 3.33

https://www.drupal.org/project/bootstrap/releases/8.x-3.33 →

Wow, feels great to run up against an issue and see it being resolved in real time.
Patch #270 solves the issue for me as well on version 2.0.9, thanks.

Unfortunately in #227 ✨ Drastically improve the linking experience in CKEditor 5 Needs work the merge request is closed.
I can't reopen it in https://git.drupalcode.org/project/drupal/-/merge_requests/10361. Maybe someone with some extra permissions?

@wim leers : I have this PHP warning in my logs:
Warning: Undefined array key 2 in Drupal\big_pipe\Render\BigPipe->sendPreBody() (line 311 in /www/web/core/modules/big_pipe/src/Render/BigPipe.php)

Created a MR for this issues, but tests are still needed.

It may make sense to drop months in favor of weeks only because a month is the only duration option that is not consistently the same and could lead to bad assumptions and potential end user confusion since we're always converting into seconds.

Rebased against latest head and moved changes oops hook files

catch → changed the visibility of the branch drupal_3317769-drupal-3317769-11.x to hidden.

catch → changed the visibility of the branch 3317769-link-suggestions-minimum-2 to active.

catch → changed the visibility of the branch 3317769-link-suggestions-minimum to hidden.

catch → changed the visibility of the branch 3317769-cke5-entity-link-suggestions-11.x to hidden.

catch → changed the visibility of the branch drupal-3317769-11.x to hidden.

catch → changed the visibility of the branch 3317769-link-suggestions-minimum-2 to hidden.

https://git.drupalcode.org/project/drupal/-/merge_requests/10036 is the merge request to be working from here, it is already against 11.x and just needs a rebase. I'm going to hide all the other branches here now.

@smustgrave I have created a merge request for 11.x compatibility (MR10351), please check. There is a JSLint issue reported, will check and post further updates. The additional MR created is closed.

Thanks for updating the MR. Can't immediately see any changes outside of the resolved conflict, so fine with keeping RTBC.

There are @todos in the change record so tagging for an update.

The last patch fails on 10.2.7 as LinkItem now provides it's config via annotation.

Attached patch from MR.

ramprassad → closed merge request !10361

I'm facing a similar issue that looks related. If I edit a page published, any field like body, and save it as a Draft the menu configured lost the parent. When we publish that Draft the menu link will be in the wrong place at the main menu since it has no parent.

Any solution/suggestion for this issue? The client likes configure the menu links in the nodes and all the time they save a Draft the menu links lost the parent.

I've re-based MR27 onto the latest release so it should be mergeable again and the diff should apply to 2.2.0-alpha1.

Is there anything else I can do to try and get this one merged in? We've been using it for several months now across a few sites and it's been working well.

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

Has this been resolved? Is there a patch one can apply?

I'm using this patch almost 7 years in multiple projects. I don't think it makes any sense to block this issue, because of missing tests.

Green again! Kept it on RTBC...

Rebased again, due to random test failure fixed in upstream: https://www.drupal.org/project/drupal/issues/3478621#comment-15875919 📌 Add a filecache-backed attribute parser Active

@ramprassad why are you opening other MRs when 10036 is pointing to the right branch? Those additional ones should be closed please

The changes in MR #127 are applied successfully.
Now, the slogan is visible in all the cases:

Show slogan only

Show slogan and name

Show slogan and logo

Show slogan with name and logo

Done the rebase, Now the MR passed, and hope it will apply to the latest version of the Drupal core.

Looks good so far, we only have those changes in the MR that we should be, no changes on ResourceTestBase that was fixed in the spin-off issue.

❯ git rebase origin/11.x
Auto-merging core/.phpstan-baseline.php
Auto-merging core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
CONFLICT (content): Merge conflict in core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
error: could not apply 07987b49a5... Cherry-picked "Improve Dynamic Page Cache header assertions in JSON:API tests"

I am going to use git merge origin/11.x instead of rebase in a hope that we do not need to restart the review process here.

Now that ✨ Improve Dynamic Page Cache header assertions in JSON:API tests Needs review is in 11.x, this needs a rebase because the MR does not apply anymore, because it had a cherry picked version. Maybe I just drop the cherry picked commit and see what happens...

Re #4: Thanks for the review! Added a draft change notice and the in-code deprecation. The merge request already adds a bunch of assertions to EntityAddUITest so what test coverage are you looking for exactly?

Re #5: I understand the use-case totally, but given that it's already hard enough to find support for this very targeted issue I don't believe broadening the scope is wise at this point. Feel free to post a separate MR (here or in another issue) for your use-case but I will continue with the current approach for now, sorry.

Updated my fork to handle duplicate aliases caused by altering path alias languages.

Also, I feel like this is actually a bug report, because it's impossible to create a new alias in a different language without overriding the fallback alias. The title could probably be updated too but I'll leave that alone for now.

Hmmm both my MR and patch #10 will result in duplicate aliases being created every time you save a node if you do something like this in hook_pathauto_alias_alter

if ($context['language'] === \Drupal::languageManager()
      ->getDefaultLanguage()
      ->getId()) {
      $context['language'] = LanguageInterface::LANGCODE_NOT_SPECIFIED;
    }

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

#260 is the correct patch. This is what should be up for MR.

Now if you are leveraging this patch 🐛 Entity reference field View output is not used for selected entity display Needs work first from this issue 🐛 Entity reference field View output is not used for selected entity display Needs work , the attached patch works.

Probably makes things more confusing. Sorry... just figured it was worth mentioning. Not sure how these two major issues will collide in the future.

There is a merge conflict. May need a new branch based on 11.x.

Rebased, pipeline successful.

Changing to 'Needs review'.

The pipeline is all green and test coverage works: a failing test is confirmed.

@oily The are dozens different render elements in Drupal core. The MR currently covers just a few ones.