Breadcrumbs for Search module should match tab titles

Good catch, thanks for reporting it!

Will need a _title entry in the routing defaults

Actually, this text mismatch is a UI bug.

Note: this does retranslate the titles twice.

Rerolled.

7: 2099391.patch queued for re-testing.

Those look like real test failures (SearchPageText title fails)

Test added.

Upps without debug()

Can you please post a screen shot of what the Search pages for Node and User look like, with the browser title bar and breadcrumbs included, before and after the patch? Because it looks like the test was changed so that it is not expecting the word "Search" in the page title. That is not right -- I think we still want the page title to say "Search" somewhere in it, not "Content"? The only thing that should be changing is the breadcrumbs, right?

@jhodgdon Absolutely, you have the reason. So the problem is the patch and not the test.

Screenshots attached.

I'll take a look at this...

Thanks

We can indeed fix that without having to work on the tests.

Can we have another before/after screen shot please?

This is exactly what I was searching for :)

getTitle() method at TitleResolver.php was adding the same title for the breadcrumb and for the page title...

Screenshots attached.

OK then. It looks like this latest patch passes all the tests without any test modifications, and the before/after screen shots show that it does solve the issue that was reported here. Let's get it in.

Thanks!

16: 2099391.patch queued for re-testing.

Committed 71d9179 and pushed to 8.x. Thanks!

I almost accidentally reverted this in another issue that I was rebasing. Why wasn't test coverage added here? This was a regression after all.

If someone wants to open a follow-up, that's fine.

I guess we should have a test that verifies the word "Node" does not appear on the screen when you go to the search page?

I think this should do it. I've uploaded both a patch with a new test, and a second patch that also comments out the new lines added in this issue, which should fail if this is a valid new test.

That test did not fail on the right lines. :( Perhaps breadcrumbs are not visible during the test. We should probably open a new issue and I don't have time to work on it today.

23: 2099391-add-test-23-without-previous-patch-should-fail.patch queued for re-testing.

I have no fails at the views test...

Changed the place for the assertions.

Let's take a look at the test.

Perfect!

Thanks for fixing that marthinal! :) I agree, this test is working as we hoped and should be committed.

Committed/pushed to 8.x, thanks!