HTML character decoded on alternate field

Reviewed the patch in #2 (alternate_field_html_decoded-3568508.patch) against 2.x (da98433) on Drupal 11.4.4 / PHP 8.5.5. Confirmed with an alternative_title_field holding Tom & Jerry: before the patch the crumb rendered Tom & Jerry, after it Tom & Jerry. ✅

The cause is that TitleResolver::getTitle() returns a plain string, so neither decoding branch in EasyBreadcrumbBuilder::getTitleString() (the MarkupInterface and #markup render-array cases) ever applies to the alternative title field.

No security concern: breadcrumb titles are passed as plain strings to Link / Link::createFromRoute(), never wrapped in Markup::create() or a #markup key, so Twig autoescaping still handles the output. Decoding after Xss::filter() is not exploitable here, and it matches the Html::decodeEntities(Xss::filter(...)) pattern already used in EasyBreadcrumbBuilder.php (lines 386, 838, 846). Coding standards are fine.

Suggestions

• Consider decoding in getTitleString() instead — an is_string($title) branch would cover every string-returning title source rather than only the alternative field, and avoids a fourth decode call site.
• No test coverage. A kernel or functional assertion on an alternative title field value containing & would lock this in — #3056018: HTML character decoded fixed the same class of bug without a test, which is how it resurfaced here.
• Possible conflict: #3354766: Support multiple alternate title field names rewrites this exact block. Whichever lands second should keep the decoding.

Testing produced with the assistance of an LLM.

Environment

• Drupal: 11.4.4
• PHP: 8.5.5
• Database: MariaDB 10.11.16
• DDEV: v1.25.2
• Easy Breadcrumb: 2.x (commit 5bfef4e)
• Browser: Chrome

Prerequisites

• Easy Breadcrumb enabled with the breadcrumb block placed, and the Basic page content type available.
• Add a plain text field field_breadcrumb_title to the Basic page content type.
• Point the module at that field:
  

  ddev drush config:set easy_breadcrumb.settings alternative_title_field field_breadcrumb_title -y

• Create a node aliased /amp-parent whose field_breadcrumb_title value contains an ampersand, e.g. Tom & Jerry.

Steps

1. Apply the fix from MR !221: EasyBreadcrumbBuilder::getTitleString() now runs plain strings through Html::decodeEntities(Xss::filter()) as well, instead of only handling MarkupInterface objects and #markup render arrays.
2. Rebuild caches: ddev drush cr
3. Visit /amp-parent and look at the second breadcrumb segment, before and after the fix.
4. Run the module test suite, including the extended EasyBreadcrumbAlternateTitleFieldTest.
5. Check that admin pages whose titles come from route title callbacks still render correctly, e.g. /admin/structure/menu/manage/main/add and /admin/structure/types/manage/page/fields.

Expected results

• The breadcrumb segment reads Tom & Jerry.
• The new test testWithHtmlCharacterReference fails on unpatched 2.x and passes with the fix.
• No change on the admin paths, which exercise the MarkupInterface and #markup branches that the MR rewrote.

Actual results

Before the fix the crumb was double-escaped: the markup contained Tom & Jerry, so the page showed Tom & Jerry. After the fix the markup is Tom & Jerry and the page shows Tom & Jerry, as expected.

The whole module suite is green with the fix applied (22 tests, 201 assertions; the reported deprecations come from the surrounding site, not from the module). The added testWithHtmlCharacterReference does cover the bug: reverting only src/EasyBreadcrumbBuilder.php to 2.x and rerunning makes it fail with Behat\Mink\Exception\ElementHtmlException: The string "Page & Test" was not found in the HTML of the element matching css "#block-breadcrumb li:nth-child(2)", while the rest of the suite stays green.

No regression on the rewritten branches: /admin/structure/menu/manage/main/add renders … / Main Navigation / Add Menu Link and /admin/structure/types/manage/page/fields renders … / Basic Page / Manage Fields, identical before and after the fix. Note that strip_tags() was dropped from the MarkupInterface branch in favour of Xss::filter(), so allowed tags now survive into the segment text instead of being removed; the segments are still passed to Link as plain strings and escaped by Twig, so this is a display detail rather than a security change, and no core admin title hit it in this test.

One small remark: testWithHtmlCharacterReference never sets ALTERNATIVE_TITLE_FIELD itself — it relies on the module's shipped default alternative_title_field: 'field_breadcrumb_title' in config/install/easy_breadcrumb.settings.yml. It works, but setting the config explicitly in the test would make it independent of that default.

Testing produced with the assistance of an LLM.

Yes, this looks good.

csakiistvan,

Your testing is appreciated and super welcome, and has been correct in finding todos. But, your AI is also being too detailed which gives us a lot to read. Please make sure your results paragraphs are super clear on whether the issues are good to go or they need more work. If they are good to go, and they need more work, please open a new issue with the more work part in that new issue.

Thanks the suggestion @greg boggs, I refine the workflow so that the end result is more quickly understood.

Thanks @bekro @csakiistvan @greg-boggs :3