Problem/Motivation

1. Install the experimental Help Topics module.

2. Clear the cache. Visit an admin page and note that the breadcrumb is correct.

3. Go to admin/help and click on a help topic link.

4. Note that the breadcrumb there is Home > Administration > Help (also correct).

5. Now visit any other admin page. The breadcrumbs remain Home > Administration > Help on all pages. Not correct.

The reason is that the HelpBreadcrumbBuilder class does not set cache context correctly, so the Block system thinks that the breadcrumb block calculated for Help Topic pages is fine to use for all other pages.

Proposed resolution

Add cache context to the help breadcrumb, so that it will not be used for other pages.

Remaining tasks

Patch will be attached in first comment. Test included. Needs review/commit.

User interface changes

Breadcrumbs for admin pages will not be taken over by Help Topic breadcrumbs.

API changes

No.

Data model changes

No.

Release notes snippet

None.

CommentFileSizeAuthor
#2 breadcrumb-code-plus-test.patch2.22 KBjhodgdon
#2 help_breadcrumb_test_only_fail.patch1.46 KBjhodgdon

Comments

jhodgdon created an issue. See original summary.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new help_breadcrumb_test_only_fail.patch1.46 KB
new breadcrumb-code-plus-test.patch2.22 KB

Here is a test-only patch that fails, and a test-with-code patch that passes. Simple fix. We cache based on the URL minus the last level, just as the System module path-based breadcrumb builder does.

The last submitted patch, 2: help_breadcrumb_test_only_fail.patch, failed testing. View results

misc’s picture

misc commented
Status: Needs review » Reviewed & tested by the community

Looks fine for me!

oknate’s picture

oknate
Location Greater New York City Area
commented

Nice find! I think this comment could use some improvement:

+ // Verify that another page does not have the help breadcrumb.

Maybe something like: "Verify that the HelpBreadcrumbBuilder class sets the cache context correctly. When visiting a new page it should have a new breadcrumb."

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed 
+++ b/core/modules/help_topics/src/HelpBreadcrumbBuilder.php
@@ -43,6 +43,7 @@ public function applies(RouteMatchInterface $route_match) {
+    $breadcrumb->addCacheContexts(['url.path.parent']);

neat! didn't know about this one

Committed 00d8daf and pushed to 8.8.x. Thanks!

  • larowlan committed 00d8daf on 8.8.x 
    Issue #3079595 by jhodgdon: The Help breadcrumb builder does not set...

Status: Fixed » Closed (fixed)

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