View titles in breadcrumb and metatag title don't get properly translated

Triaging

issue still present in 8.2.x
steps to reproduce accurate

Thanks @dbyers55 for confirming the issue! Updating issue credit.

Having the same problem with Drupal 8.1.

It appears that it is getting the title from the original view and not checking for the translated title. I've been running into similar issues with content links doing the same and only getting the original node url and not checking for the translated one. And with D8 being so different from previous versions, I'm not even sure where to start looking for where the bug could be makes it very frustrating.

Ok I think I've found the problem.

It seems that Views Breadcrumb Titles are not using the new "Translate View" tab to look up the translated string. Instead it is still searching for it in the User interface translation section (admin/config/regional/translate).

I also came across that when creating a new view that it will be stored automatically as your site's default language (In my case default is Japanese and 2nd language is English). Which confused the hell out of me as I thought it would be done as the interface language, so was only adding translations from English to Japanese, not the other way round. Making me have the Japanese word under the English column in User interface translation section (admin/config/regional/translate) and then having the English word under the Japanese column. To solve this I ended up temporary setting English as the default language to create the view and now my view breadcrumbs are displaying correctly.

To make this less confusing, I think that it would be much better that views breadcrumbs get it's title from the new "Translate View" tab section and not the old "User interface translation" section. And secondly, adding a language dropdown when creating the view so we don't get confused to which language it is in.

Anyways, hope that helps you other guys having the same problems.

Thanks @nickolas.wilson for looking into this and documenting that workaround. It definitely is not intended for that workaround to be required!

@alexpott, @catch, @webchick, @effulgentsia, and I discussed this issue and agreed it is major per https://www.drupal.org/core/issue-priority#major-bug. It interferes with normal site visitors' use of the site (by presenting breadcrumbs in the wrong language), which makes it major even with a workaround being available.

A next step for the issue will be to create a failing test demonstrating the bug.

Active since there is not yet a patch.

unasigning myself from this issue.

I'm taking a look at this on DrupalCon Dublin 2016.

Well, I am stuck, but wanna share what I tried in order to track down the issue.

The Breadcrumb builder is receiving the the wrong translatedMarkup already:

So I tried to drill down where it is coming from. I noticed that in \Drupal\Core\StringTranslation\TranslationManager::getStringTranslation, none of the sortedTranslators returns anything via $translator->getStringTranslation().

The breadcrumb's title string is being passed through these two Translators:
- Drupal\Core\StringTranslation\Translator\CustomStrings
- Drupal\locale\LocaleTranslation

This is where I got stuck with my limited D8 knowledge.

What I don't know is...
- ... should there be another Translator service in that list for it to work?
- ... are the strings I translated in Views UI supposed to be picked up by on of the Translators above?

I hope that helps anyone to track it down further.

I had a look at this issue, and it looks like the PathBasedBreadcrumbBuilder is using TitleResolver->getTitle() to add the desired string to the breadcrumb.

I uploaded a quick patch to fix this, however I am not sure if this is the right way to go.

The main idea is that we need to load the title from the configurations, because the Route that is generated doesn't contain the correct title in the default values.

My fix loads the view and thus the correct title, but maybe we need to register a _title_callback for the Route, or perhaps the problem is solvable by going deeper and by generating correct default values for the Route object.

Maybe one "solution" would be to be able to specify _title_langcode or so. On the other hand you are right, maybe we have to use a title callback and at least load the translated version of the view title.

Is there a scenario that would require _title_langcode or a situation where we are not taking the language from the current context?

Hi all! As I saw this issue was marked 'needs work' I followed up on @benelori's solution.

Investigated more on how the routes are set for the views. Drupal\views\Plugin\views\display\PathPluginBase::getRoute is generating the dynamic route for a given view and display. So the title gets set to the default language and will be converted to a TranslatableMarkup. But the problem is that the translated title is stored as configuration translation, so the two never get connected and the title can be found in the user interface translation untranslated.

So to solve the issue in a better way I've implemented the suggested _title_callback. I've placed the callback method in the Router\ViewsPageController, but I am not quite sure if this is the right place for this.

I guess this issue could also be solved by automatically creating the user interface translation when the configuration translation for the view is added, but that would mean duplicate data, or maybe creating some kind of bridge between the two types of translation. It might also be solvable by making the TranslatableMarkup to also use config translation in some way, but that would complicate things a lot.

Put back the default _title that I should've not removed.

Reordered displaying of files.

Hi,

Can you please write a test for this new method that you added to the controller?

You know for issues like this its always better to start with the test first and let the implementation be done later. This makes the implementation side flexible and not be tight to one, and really just one solution.
In general I'm a bit worried about the potential performance problems.

1. +++ b/core/modules/views/src/Routing/ViewPageController.php
   @@ -61,4 +62,24 @@ public function handle($view_id, $display_id, RouteMatchInterface $route_match)
   +   * The parameter names of this method are explicitly named this way as they
   +   * are discovered by \MethodReflection and automatically set from matched
   +   * route attributes.
   +   *
   

   IMHO we don't have to document that. It is the same for every title callback.

2. +++ b/core/modules/views/src/Routing/ViewPageController.php
   @@ -61,4 +62,24 @@ public function handle($view_id, $display_id, RouteMatchInterface $route_match)
   +   * @param $view_id
   +   *    The id of the view.
   +   * @param $display_id
   +   *    The id of the display from the view.
   

   Let's document that its a string parameter.

Created a test for this issue. I couldn't find (I believe there is not) a suitable class to add the test to so I created one. As I notice there is very few tests for config translations.

Adding a patch with only the test so it can fail and the patch with the test, updated according to @dawehner.

@dawehner about the performance: node also uses title callback to get it's title. We could make my patch better if we only added the title callback if the website is multilingual. But for this we need the language manager in the PathPluginBase class which is extended and constructed by other classes, so it requires some changes:

if ($this->languageManager->isMultilingual()) {
     $defaults['_title_callback'] =
     'Drupal\views\Routing\ViewPageController::getTitle';
}

PS: Just learned that I shouldn't name an interdiff with diff extension :D

Set file ordering and status.

Re #16

The main idea is that we need to load the title from the configurations, because the Route that is generated doesn't contain the correct title in the default values.

I wonder if this is related to #2835018: The content type name in the backend breadcrumb should be Human name and #2513570: Changing name (label) of content type is not reflected in breadcrumb link text where we learn that admin breadcrumbs are wrongly displaying machine names not titles.

+++ b/core/modules/views/src/Plugin/views/display/PathPluginBase.php
@@ -129,6 +129,7 @@ protected function defineOptions() {
       '_title' => $this->view->getTitle(),

Let's remove the title completely. It's simply incorrect and can lead to translation errors if modules check that directly (instead of using the TitleResolver).

Seems that was put back in #23 to avoid the test failures in #20 but that just means we need to fix the tests. Seems it's only unit tests that are affected, so that's should be doable.

Marked #2844506: View route titles are double translated as duplicate.

Tried to rewrite the unit tests that fail with the _title taken out, but it seems real hard, as I am using Views::load which requires entity manager .. so would mean many mock objects. Maybe I am think about this wrong. Should we be testing the functionality of the ViewPageController::getTitle in the PathPluginBaseTest or just get a mock for it that returns the right title?

Just a rebase of #28 for 8.3.x and the patch without the "_title" to see how many tests fail now (#20 was 7 months ago)

As a side effect this bug is polluting the database by calling TitleResolver::getTitle and passing through t() non-english strings.

I had a look at the unit tests that are failing and I feel we should not mock the Views page controller or anything since that is not what we are testing here.

I changed the unit test so the check for the callback 'Drupal\views\Routing\ViewPageController::getTitle' to be set just like the
testAlterRouteWithAlterCallback test in that same file.

There is no unit test for the page controller
http://cgit.drupalcode.org/drupal/tree/core/modules/views/tests/src/Unit...
and I don't think there should be one as the the one that this patch creates good coverage.

I had some similar issue with views display titles. The views display title is not translated in the HTML head section. I can confirm that this patch solves this issue also. Good work!

Faced absolutely the same problem on multilingual site. Patch from #40 fix it! Thank you guys!

+++ b/core/modules/views/src/Plugin/views/display/PathPluginBase.php
@@ -129,7 +129,7 @@ protected function defineOptions() {
       '_controller' => 'Drupal\views\Routing\ViewPageController::handle',
-      '_title' => $this->view->getTitle(),
+      '_title_callback' => 'Drupal\views\Routing\ViewPageController::getTitle',
       'view_id' => $view_id,

Given that these might have performance implications could be maybe check somehow whether the view will have a dynamic title somehow? In case it doesn't have one, could we unset the _title_callback?

1. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,113 @@
   +class ViewTranslationTest extends ViewTestBase {
   

   \Drupal\views\Views\ViewTestBase is deprecated
   Use \Drupal\Tests\views\Functional\ViewTestBase.

2. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,113 @@
   +    $this->assertEqual($titleResolver->getTitle($request, $route),
   ...
   +    $this->assertEqual($titleResolver->getTitle($request, $route),
   

   assertEqual also deprecated, use assertEquals.
   And don't forget swap first tho arguments.

   Looks like you forgot to add use Drupal\Tests\views\Functional\ViewTestBase; to Use section

I think we should still resolve #44

As we know, drupal routing system always wait English string in _title.

View can be non-English in many cases:
site have only one, non-english language.
site have multiple languages enabled.

Also t() function designed to translate interface strings(static text with placeholders, etc.)

Since view title is config value, it shouldn't be translated via t().
In this logic we shouldn't use _title at all.

As #44, I think this need some investigation, profiling. I do some debug around this and I see that calls to
Drupal\views\Routing\ViewPageController::getTitle always use cache.

Another problem is title overriding by contextual filters.

View should be executed to get actual title.

Patch #40 works fine!

#40 worked for me as well (cache rebuild was needed for the patch to take effect).

#40 works for me too, after cache rebuild.

Confirming #40 works after a cache rebuild too.

Patch #40 works for me

As #44, I think this need some investigation, profiling. I do some debug around this and I see that calls to
Drupal\views\Routing\ViewPageController::getTitle always use cache.

=

Thanks for the patch. #40works great!

The patch in #40 still has one issue: Drupal\views\ViewExecutable::getTitle returns a string but strings are escaped in twig so the views title appears escaped in the breadcrumb (e.g. "News & Events").

Added a patch that outputs the config string as a (safe) Markup object.

Added a patch to filter the title to strip dangerous tags:

+    return Markup::create(Xss::filter($view->getTitle()));

Tested and approved.

Thanks for the patch mpp.

1. +++ b/core/modules/views/src/Routing/ViewPageController.php
   @@ -61,4 +64,22 @@ public function handle($view_id, $display_id, RouteMatchInterface $route_match)
   +    return Markup::create(Xss::filter($view->getTitle()));
   

   The Xss::filter() is super important and therefore we need some test coverage to prove this is implemented. Could have easily been forgotten. Really nice to see this has been considered though @mpp++

2. Some coding standards to fix...

   FILE: ...al8alt.dev/core/modules/views/src/Routing/ViewPageController.php
   ----------------------------------------------------------------------
   FOUND 3 ERRORS AFFECTING 3 LINES
   ----------------------------------------------------------------------
    71 | ERROR | [x] Parameter comment indentation must be 3 spaces,
       |       |     found 4 spaces
    73 | ERROR | [x] Parameter comment indentation must be 3 spaces,
       |       |     found 4 spaces
    76 | ERROR | [x] Return comment indentation must be 3 spaces, found
       |       |     4 spaces
   ----------------------------------------------------------------------
   PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
   ----------------------------------------------------------------------
   
   Time: 54ms; Memory: 8MB
   
   
   FILE: ...pal8alt.dev/core/modules/views/src/Tests/ViewTranslationTest.php
   ----------------------------------------------------------------------
   FOUND 6 ERRORS AND 5 WARNINGS AFFECTING 11 LINES
   ----------------------------------------------------------------------
      5 | WARNING | [x] Unused use statement
      6 | WARNING | [x] Unused use statement
      7 | WARNING | [x] Unused use statement
      8 | WARNING | [x] Unused use statement
      9 | WARNING | [x] Unused use statement
     34 | ERROR   | [x] Data types in @var tags need to be fully
        |         |     namespaced
     41 | ERROR   | [x] Data types in @var tags need to be fully
        |         |     namespaced
     76 | ERROR   | [x] Data types in @var tags need to be fully
        |         |     namespaced
     80 | ERROR   | [x] Data types in @var tags need to be fully
        |         |     namespaced
     84 | ERROR   | [x] Data types in @var tags need to be fully
        |         |     namespaced
    113 | ERROR   | [x] Expected 1 newline at end of file; 0 found
   ----------------------------------------------------------------------
   PHPCBF CAN FIX THE 11 MARKED SNIFF VIOLATIONS AUTOMATICALLY
   ----------------------------------------------------------------------
   

Patch #40 works for me

Fixes for the coding standards issues alexpott identified in #65.

Hello,

I have tested the patch from comment 67 on a Drupal core 8.7.5, It fixes the bug.

Thanks!

It seems Easy Breadcrumb might be affected by this. Does anyone have any advice for me on?

https://www.drupal.org/project/easy_breadcrumb/issues/3072117

Hi,

I tested the patch from comment #67 on a Drupal core 8.7.6, It fixes the bug.

Thanks!

Patch on #67 works for me on a Drupal core 8.7.3.

Thanks!

Hi,

I have reviewed the patch #31 and it worked as designed.

I had enabled the translation-related modules and add one more language(French) than English And enable translation for page content type and created a translated view for page content type.

And created a node for page content type and given the path of page same as view.

On page view, the name of the view is being translated in the breadcrumb.

Below is an output screenshot in the English language.

Below is an output screenshot in the french language.

Thanks,
Ren

Patch #67 worked for me using D8.7.10

In my case I was using a german view and didn't get the english title when using {{ drupal_title() }} from Twig tweak module (which is also using \Drupal::service('title_resolver')->getTitle())

1. +++ b/core/modules/views/src/Routing/ViewPageController.php
   @@ -63,4 +66,22 @@ public function handle($view_id, $display_id, RouteMatchInterface $route_match)
   +   * @return false|string
   +   *   The title of the display of the view.
   ...
   +    return Markup::create(Xss::filter($view->getTitle()));
   

   This doesn't return FALSE as far as I can see. Also doesn't return a string. It returns a Markup object. Also let's use \Drupal\views\Render\ViewsRenderPipelineMarkup as that's View's special markup object for it's funky rendering.

2. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,108 @@
   +class ViewTranslationTest extends ViewTestBase {
   

   Tests need a $defaultTheme property as of 8.8.x

3. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,108 @@
   +    $this->languageManager = \Drupal::languageManager();
   +    $this->otherLanguage = $this->languageManager->getLanguage('ro');
   

   Putting services as protected properties on a test class has prove problematic in the past. Let's not do that.

4. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,108 @@
   +   * Test if the view route title gets translated.
   

   Tests that the view route title is translated.

5. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,108 @@
   +    /** @var \Drupal\Core\Routing\RouteBuilderInterface $routeBuilder */
   +    $routeBuilder = \Drupal::service('router.builder');
   +    $routeBuilder->rebuild();
   

   Can be \Drupal::service('router.builder')->rebuild()

6. +++ b/core/modules/views/src/Tests/ViewTranslationTest.php
   @@ -0,0 +1,108 @@
   +    $this->assertEqual($titleResolver->getTitle($request, $route),
   +      'Title EN', 'The default title is in the default language.');
   ...
   +    $this->assertEqual($titleResolver->getTitle($request, $route),
   +      'Title RO', 'The route title gets translated when language set.');
   

   Should use $this-assertEquals() and the expected value should go first.

Patch #67 worked for me using D8.8.1

In my case I have a translated view an am using the [current-page:title] token via the metatag module to render the HTML title tag.

Addressed #47 and #75.

+++ b/core/modules/views/src/Routing/ViewPageController.php
@@ -63,4 +66,22 @@ public function handle($view_id, $display_id, RouteMatchInterface $route_match)
+   * @return \Drupal\Component\Render\MarkupInterface
+   *   The title of the display of the view.
+   */
...
+    return ViewsRenderPipelineMarkup::create(Xss::filter($view->getTitle()));
+  }

Technically this just wraps \Drupal\Component\Render\MarkupTrait::create() which returns

   * @return string|\Drupal\Component\Render\MarkupInterface
   *   A safe string.

However, since #75.1 specifically requested this to be documented as a MarkupInterface I suggest we leave this up to the maintainer if this should be updated.

Patch works as expected. Test coverage tests the title value, but the issue summary mentions the breadcrumb title. Perhaps the test coverage should be extended to cover this element?

Massively redid the test coverage because it wasn't testing what we were hoping, removing the call to $request->setLocale($languageManager->getLanguage('ro')->getId()); would still cause the test to be green, so the language wasn't doing anything. Adding a new test-only patch because its pretty different then the original test.

• Made it into a full functional test (was a kernel test in functional test clothing)
• Really tests the translation process now
• Added coverage for the breadcrumb translation
• Added coverage for XSS titles per #65.1
• Added coverage for page cache to make sure we aren't breaking caching when using the title callback with views

Addressed #78. Since the parent function can return a string (when an empty string is passed), the child function should match that.

I'm removing the 'needs profiling' tag, we have coverage for caching and if just using a title callback causes massive performance hits, then I feel that would be a problem with title callbacks in general and not just for views and should probably be addressed outside views.

Quick clean up

The test coverage is looking much better now, thanks! Setting back to RTBC.

Looks like the latest patch is not apply to 9.1.x -- we need a reroll here. Thanks!

I am interested in this getting fixed so here is a reroll for 9.1.x. It was just changing some assertSame(FALSE, $var) to asserTrue or assertFalse.

Fix the test

Removed "Needs reroll" tag

Thanks @rodrigoaguilera! Setting back to RTBC.

Looks ok!

Made a separate patch for Drupal 8.9, since patch at #87 doesn't apply because of #3082415: Replace assert(Not)Same/Identical() on booleans with assert(Not)True/False() in PHPUnit tests which is only in D9.

This is the same patch than the one at #87, only with different context lines. Because of that, Interdiff tool did not work between the 2 patches.

We need to fix the 9.x patch to not use assertLink / assertNoLink

Remaining self deprecation notices (4)

2x: AssertLegacyTrait::assertLink() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->linkExists() instead. See https://www.drupal.org/node/3129738
2x in ViewTranslationTest::testViewTitleTranslation from Drupal\Tests\views\Functional

2x: AssertLegacyTrait::assertNoLink() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->linkNotExists() instead. See https://www.drupal.org/node/3129738
2x in ViewTranslationTest::testViewTitleTranslation from Drupal\Tests\views\Functional

Fixed patch for 9.x. Applied @alexpott suggestions.

Note: as per #2787051-51: Views contextual filter's title override is invisible to TitleResolver, this patch does not address the issue of overridden titles, such as with Contextual filters. Would it be worth including a fix for these as well in this patch? I have a new patch ready with these fixes.

This patch is the one at #93 and additionally includes fixes for overridden Views titles, from #2787051: Views contextual filter's title override is invisible to TitleResolver.

I think it would be worth adding a few test cases for overridden titles and translations (with Contextual filters), but I'm lacking time to dive into it right now.

Drupal 8 version of the patch at #95

Patch #96 worked for me

Sorry I need this patch for D8.8.8, here it is rerolled from 8.9.x patch 96
Not sure why this is marked as needs tests, the patch already has new tests.

*EDIT* editted twice, YES THIS PATCH WORKS FOR US! thanks so much!
#2665980: [current-page:title] doesn't integrate well with views

#95 Works for me. Thanks.

Has tests, great fix! Using 99 on 8.8.10 but other reports of patch 95 working also for D9.1.x.

Needed this patch for a view page that has a translated menu link title, with this patch the correct breadcrumb title shows up in all of our languages.

Thanks!

This is an important fix because without this fix the incorrect title can get cached in the breadcrumb for all languages.

Please commit this fix, it's actually a significant improvement and very tough to debug, would be best to just have this fix immediately in the next release of core. Save so many people the time of debugging this and searching for a solution or trying to implement one. Trust me on this one. Has tests, please it's been 4 years lets just get this in, it is a win-win-win.

Stuff got added in #95 that never got test coverage and never really got a full explanation why that was pulled into scope. If we have a separate issue to address this, why are we pulling it in here?

I would suggest we roll this back to #93 (and RTBC that?) and not scope-creep on this. If there is a valid reason to add to the scope here, we need to explain why and add coverage for it .

@Lendude, this simplified version of the patch should do the trick, we should move forward with it and then open a new issue for the other unrelated bits that were adressed in the larger patch from 96.

For the sake of expediting progress (let's please not wait another 4 years) and also simplicity I agree with your plan to move forward and then open new issues for whatever was missed that is included in patch 96.

PostgreSQL 12.1 fails can be ignored, has nothing to do with this patch, HEAD does not pass testing on postgresql 12.1

Let's give @Lendude another chance to review

#104 Works well on Drupal 9.0.7. RTBC+1.

#104 seems to have lost all the new test coverage that was still present in #93, can we add that back?

Here is the correct reroll of patch 93

#109 works well on my 9.0.x install.

#109 seems to contain the right code now, but we do need to get them green :)

Minor change for the D9.0 and D9.1 patches (see interdiff)
public to protected modules property

D9.1 and D9.0 patch is the same.

New 8.9.x patch also

Let's focus on 9.1.x and 9.0.x

ok @lendude patch 115 has the fix, there was a recent change to 9.1.x that requires that the $modules property be declared as protected, please look at the interdiff, it's pretty simple, I think this should be moved back to RTBC

lets get this into 9.1.x and 9.0.x soon

I accidentally queued php 8.0, please ignore that.

thanks

Ok , D8.9.x issue was related to the interdiff which should only be included for D9

I reversed the interdiff part of the 8.9.x patch

this should pass testing with 8.9.x

Let's make the fix patch the most recent patch on the issue. So this is just a re-up of the 9.1 patch in #115, no changes.

For TitleOverrides we have #2787051: Views contextual filter's title override is invisible to TitleResolver as a follow up.

Everything seems to be back in place, back to RTBC.

Random test failure. Back to RTBC.

@Lendude, is there anything left to do here other than to commit and push the patch (and create followup issues for leftovers)?

Unrelated fail, back to RTBC

When I apply this patch the described problem is solved, but I lose the breadcrumb when creating/editing content

Looks like #128 needs to be addressed - we should add test coverage if that's reproducible.

confirmed, looking into it now.

catch, iiRealXz, I confirm that the patch causes the described regression illustrated in #128

Need to not only fix the patch but also add core test coverage for this illustrated breadcrumb which should be 'Node'

expected breadcrumb in illustration from #128 is :
Home » Node » Add content

with that said, we still need to also fix what is discribed in the issue title (without causing a regression like this).

I tried reproducing #128 too, but couldn't on a vanilla Umami install. Tried with Seven and Claro themes. I just get the breadcrumb but without the double » shown in the screenshot.

Tried to add easy_breadcrumb cause I could see that triggering this, but no problems with that either

Seven:

Easy breadcrumb:

Am I missing a step to reproduce this?

You didn't miss a step, you did reproduce it

it should be Home » Node » Add Content » Create Basic Page

NOT Home » Add Content » Create Basic Page

the regression is that we're missing Node » from the breadcrumb when using the patch

I don't see a reference to Node with or without the patch. With or without the patch, the breadcrumbs look the same as in the screenshots I posted.

Like I said this is in a vanilla Umami install, might be different on another profile, haven't looked at that.

In the screenshot in #128 I see » » which does indicate something is there but not loaded correctly, I don't see that in my setup either

I also could not reproduce as #128 pointed.
Maybe @iiRealXz can tell us the steps do reproduce.
I'm using the standard install.

using the claro theme?

No, I'm using seven theme.

My breadcrumb looks normal. I don't think Node » is missing in my case. @lendude already attached a foto from Seven. I'm attaching one from Claro. Let's see if @iiRealXz will say something about it.

make sure you rebuild cache between patching and unpatching, I think this will reveal it.

Yes, in my portal I only have another patch applied to the core (see below link), but this has happened to me in 3 websites.

Steps to reproduce this error:

• Install this patch
• Clear caches

Details of my website:

• Drupal core: 8.9.6
• Admin theme: Seven
• Using docker

Patch: Views hardcodes exposed filter block form ID's which breaks AJAX when the same form is shown multiple times on one page

I second @paulocs "node" is not part of my installation either and I don't think it has ever been.
Using 9.1.x-dev + seven. NOT using @iiRealXz patch.

We're using menu_breadcrumb, this is probably the main difference. If I back off the patch it comes back after a cache rebuild.

So this patch behaves differently before and after patching for those using menu_breadcrumb

**EDITTED**

Patch #121 works for me with Metatag enabled

Updated the class description.

Fixed the typo.

Fixed depreciation error. Please review the patch.

#147 is good for us.

Moving to RTBC per comment #148.

#147 Works on Drupal 9.1.3. RTBC+1

#147 working for me
Thank you

This patch needs a post update function to trigger a route rebuild. I.e. it need to do something like:

/**
 * Rebuild routes to fix view title translations.
 */
function views_post_update_title_translations() {
  \Drupal::service('router.builder')->setRebuildNeeded();
}

Given this is merely triggering a rebuild I don't think an update path is necessary.

It would be good to work out what is going on with menu_breadcrumb and if it is the cause of #128 - especially as it is a well used Drupal 8 module.

Added post update function.

The patch does not apply to Drupal 9.1.6 and 9.1.7.

Reroll for 9.1.7+ from #153

Re-rolled for 9.2.x.

I am using the metatag module.

Is this the reason why view titles (only the browser title) are not translated?

Is this the right patch to solve the problem?
When will it be commited?

I tried using patch #156 with drupal 9.1.x and it works fine.

We still need to work out what is going wrong with menu_breadcrumb and why this affects it. See #128.

I've also updated the issue credit.

I've tried to reproduce #128 with menu_breadcrumb installed. Here are my efforts to reproduce this with the patch applied:
1. I create a view and added to the tools menu and I called the menu item node.
2. I created a menu item for node/add/page in the tools menu...

Here's a screenshot of my efforts:

Unless @joseph.olstad or @iiRealXz give us concrete steps on how to reproduce the issue with menu_breadcrumb I think we should commit this patch because it is obviously fixing a problem for a lot of users. I will commit this patch on 25th May unless there is more feedback as to why we should not

Perhaps the issue in #128 is caused by invalid paths? Easy Breadcrumb has a feature to print breadcrumbs when paths are invalid, menu breadcrumb doesn't, I don't think?

If the test site has no valid path at /node, then it probably won't have a linked breadcrumb to node because that path is a 404. I don't know why this patch would interact with that behavior in menu breadcrumb because this patch adds a title callback for views which is needed so that getTitle(); can return the title of the view.

We've got some placeholder code in Easy breadcrumb to get views titles until this patch is ready, but if there are issues in #128 with Menu Breadcrumb, perhaps we should follow up in the Menu Breadcrumb module (Volunteering myself to work on it if I can reproduce the issue).

I've opened #3215110: Add automated test coverage on the menu_breadcrumb module.There's no automated test coverage so there's no tests we can run either way to prove it works or is broken by this patch.

I will commit this patch on 25th May unless there is more feedback as to why we should not

I reviewed this patch and couldn't fault it.

The issue with menu_breadcrumbs shouldn't hold up with issue as mentioned above.

Could the difference be that folks have the frontpage view enabled vs not? As that is what serves the route at /node

Typically that is the one of the first things people delete when building a site.

Given May 25th has passed and this is a 5 year old major bug, I'm going to go ahead and commit it.

Committed 93d251d and pushed to 9.3.x. Thanks!

I'll discuss with other committers whether this is eligible for backport. The presence of the update hook makes it possibly disruptive.

Discussed this with @catch who felt it was better to backport this during the RC than a minor.

So backported to 9.2.x

Thanks all

Adding issue credit for @catch for the release-manager review of backporting

Because our Thunder distribution tests are failing since this was committed, I investigated a bit and I have some steps to reproduce:

1. Install Drupal 9.2.0-rc1
2. Create a Basic Page
3. On node/1 is the breadcrumb “Home”
4. Remove default front page
5. On node/1 is the breadcrumb “Home > Node”
6. Apply this patch and clear cache
7. On node/1 is the breadcrumb “Home >”

So yes, the default front page is the crucial part.

@chr.fritsch if you stop at step 5 above, what does the Node breadcrumb link to?

@larowlan
Home links to /
Node links to /node

Is there anything at /node if the view is disabled?

Doesn't seem to be anything in node.routing.yml

What I'm hinting at is that we fixed another bug by accident, a broken node breadcrumb without the view

If the view is disabled, we get a 404 page. So that all makes no sense :)

Because the link is broken, I think it's fine to leave it out. So in 9.2.0, if the view is enabled, we get 'Home >'. If the view is disabled it's just 'Home'.

I wonder if there is no title set on the front page view, if so that's the issue - can we get a follow up for that?

I can reproduce this with 9.2.x-dev. Using a fresh install, I:

1. created a Basic Page with the title "Home" and path /home
2. Set the default front page to /home
3. Visited node/add/article

This produces:

@katherined can you edit the frontpage view and add a title and see if that resolves it?

If so, I think we need a new bug here, and the bug is 'The frontpage view assumes it will be the homepage and hence doesn't have a title'