[#3563874] Updated check for when menu item titles are render arrays.

[richardgaunt]

JIRA ticket: #3563874

Checklist before requesting a review

• I have formatted the subject to include ticket number as
• I have added a link to the issue tracker
• I have provided information in section about WHY something was done if this was not a normal implementation
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works
• I have run new and existing relevant tests locally with my changes, and they passed
• I have provided screenshots, where applicable

Changed

1. Check for menu item to be an array.

Background

Comes from this conversation thread: https://drupal.slack.com/archives/C039UV0CQBZ/p1765788174230759?thread_ts=1765470743.227649&cid=C039UV0CQBZ

Screenshots

Summary by CodeRabbit

• Bug Fixes
  • Normalized menu item title handling: strips HTML from string titles, preserves non-string values, and sets a default empty title when missing to prevent warnings and ensure consistent behavior across menu items. Improves menu rendering stability. No public interface changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified menu item title normalization in _civictheme_preprocess_menu_items to defensively handle non-string or missing title values. Strings are sanitized with strip_tags; non-string titles are preserved; missing titles default to an empty string. Added a PHPMD.ElseExpression suppression comment.

Changes

Cohort / File(s)	Summary
Menu item title normalization web/themes/contrib/civictheme/includes/menu.inc	Added PHPMD.ElseExpression suppression; changed title normalization to: apply strip_tags only when title is a string, preserve non-string titles, and set missing titles to '' (empty string). No public API/signature changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Updated dependencies for security releases. #1451: Modifies the same web/themes/contrib/civictheme/includes/menu.inc logic and HTML sanitization approach.
• #3558421 Fixed menu items to remove html entities from output. #1450: Adjusts menu item title sanitization in the same file, moving to strip_tags-based handling.

Suggested labels

State: Needs review

Suggested reviewers

• alan-cole
• cbiggins

Poem

🐇 I tidy titles with a careful paw,
Strings get stripped, others stand in awe.
Missing names? I give them space,
A gentle hop keeps menus in place. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title accurately describes the main change: updating the check for when menu item titles are render arrays/renderables, which aligns with the code modifications adding normalization logic for menu item titles.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

web/themes/contrib/civictheme/includes/menu.inc (1)

30-35: Logic correctly handles renderable titles.

The is_string() check appropriately prevents strip_tags() from being called on renderable arrays, which would cause warnings. Non-string titles are preserved for Drupal's render system to handle.

However, the PR checklist indicates tests were not run. Please verify this change works correctly with both string titles and renderable titles.

Optional: Add clarifying comment

Adding a comment would help future maintainers understand why non-string titles are preserved:

+    // Preserve non-string titles (e.g., renderable arrays) for Drupal's render system.
     if (isset($item['title'])) {
       $item['title'] = is_string($item['title']) ? strip_tags($item['title']) : $item['title'];
     }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 278429e and 0b57b9b.

📒 Files selected for processing (1)

• web/themes/contrib/civictheme/includes/menu.inc

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: joshua-salsadigital
Repo: civictheme/monorepo-drupal PR: 1350
File: web/themes/contrib/civictheme/includes/primary_navigation.inc:17-23
Timestamp: 2025-08-04T18:38:53.749Z
Learning: In CivicTheme, the `expand_all_items` configuration setting is only valid for sidebar menu links. Primary and secondary navigation menus should hardcode FALSE when calling `_civictheme_preprocess_menu_items()` and should not read the `expand_all_items` configuration.

📚 Learning: 2025-08-04T18:38:53.749Z

Learnt from: joshua-salsadigital
Repo: civictheme/monorepo-drupal PR: 1350
File: web/themes/contrib/civictheme/includes/primary_navigation.inc:17-23
Timestamp: 2025-08-04T18:38:53.749Z
Learning: In CivicTheme, the `expand_all_items` configuration setting is only valid for sidebar menu links. Primary and secondary navigation menus should hardcode FALSE when calling `_civictheme_preprocess_menu_items()` and should not read the `expand_all_items` configuration.

Applied to files:

• web/themes/contrib/civictheme/includes/menu.inc

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: commit

🔇 Additional comments (1)

web/themes/contrib/civictheme/includes/menu.inc (1)

29-29: Good defensive initialization.

Ensuring below is always an array prevents potential issues in the recursive call.