Skip to content

Fix review findings: canonical URL normalization, JSON-LD escaping, ARIA state, CI coverage - #5

Merged
AlexanderWagnerDev merged 1 commit into
agent/reach-and-seofrom
claude/review-fix-prs-i207bi
Jul 18, 2026
Merged

Fix review findings: canonical URL normalization, JSON-LD escaping, ARIA state, CI coverage#5
AlexanderWagnerDev merged 1 commit into
agent/reach-and-seofrom
claude/review-fix-prs-i207bi

Conversation

@AlexanderWagnerDev

@AlexanderWagnerDev AlexanderWagnerDev commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What changed

Addresses the four review findings (qodo-code-review) posted on #2, which also line up with the SonarCloud quality gate failure on that PR (Reliability B, Security C on new code):

  • includes/header.php: normalize $canonicalPath by stripping a trailing /index.php, so pages that don't set $canonicalPath explicitly (docs, legal) emit the same canonical URL whether reached via the directory path or the index.php path.
  • includes/header.php: add JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT to the JSON-LD json_encode() call so structured data can't break out of the <script type="application/ld+json"> tag.
  • assets/js/main.js: sync the nav toggle button's aria-expanded attribute with the actual open/closed state instead of only toggling a CSS class.
  • .github/workflows/site-checks.yml: add /legal/ to the CI smoke-test route list — it's listed in sitemap.xml but wasn't being checked.

Validation

  • php -l on the changed PHP file: no syntax errors.
  • Ran the site locally with php -S and curled /, /quickstart/, /docs/, /legal/, /guides/: all return 200.
  • Confirmed the homepage's <link rel="canonical"> and JSON-LD <script> render correctly after the change.

Base branch is agent/reach-and-seo (the existing open PR #2) so this merges into that PR rather than opening a separate, conflicting change against main.


Generated by Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

… coverage

- Normalize the canonical path by stripping a trailing /index.php so
  directory and index.php URLs for the same page (docs, legal) emit
  an identical canonical link.
- Add JSON_HEX_* flags to the JSON-LD json_encode call so structured
  data can never break out of the surrounding <script> tag.
- Sync the nav toggle's aria-expanded attribute with its open/closed
  state in main.js instead of only toggling a CSS class.
- Add /legal/ to the CI smoke-test route list so it matches
  sitemap.xml coverage.

Addresses the SonarCloud quality gate failures (Security/Reliability
ratings on new code) flagged on this PR.
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: cf945069-4921-485a-8511-5b5659d0d9d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix canonical URL normalization, JSON-LD escaping, ARIA state, and CI route coverage

🐞 Bug fix ✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Normalize canonical URLs by stripping trailing "/index.php" from inferred request paths.
• Harden JSON-LD output by hex-escaping HTML-sensitive characters in json_encode().
• Sync nav toggle aria-expanded with actual open/closed state and expand CI route coverage.
Diagram

graph TD
  U["Browser"] --> H["includes/header.php"] --> C["Canonical link"] --> U
  H --> J["JSON-LD script"] --> U
  U --> M["assets/js/main.js"] --> A["ARIA expanded"] --> U
  W["GitHub Actions"] --> S["site-checks.yml"] --> R["curl smoke routes"]

  subgraph Legend
    direction LR
    _u["User agent"] ~~~ _code["Code / config"] ~~~ _out["Rendered output"]
  end
Loading
High-Level Assessment

The PR’s approach is the most direct, lowest-risk way to address the stated review/Sonar findings: normalize canonical paths at the source of truth (header.php), use standard JSON_HEX_* flags for safe JSON-in-script embedding, set aria-expanded from real state to ensure accessibility correctness, and align CI smoke routes with sitemap coverage. Considered alternatives (e.g., rewriting URLs at the server/router layer, or moving JSON-LD escaping to a separate helper) would add complexity without improving correctness for this scope.

Files changed (3) +5 / -2

Bug fix (2) +3 / -1
main.jsKeep nav toggle aria-expanded in sync with open state +1/-0

Keep nav toggle aria-expanded in sync with open state

• Updates the click handler to set aria-expanded based on whether the nav has the 'open' class. This ensures assistive technologies receive the correct expanded/collapsed state.

assets/js/main.js

header.phpNormalize canonical path and harden JSON-LD output escaping +2/-1

Normalize canonical path and harden JSON-LD output escaping

• Normalizes inferred canonical paths by stripping a trailing /index.php to avoid duplicate canonical URLs. Adds JSON_HEX_* flags to json_encode for JSON-LD so content cannot prematurely terminate the script tag.

includes/header.php

Other (1) +2 / -1
site-checks.ymlAdd /legal/ to CI smoke-test route list +2/-1

Add /legal/ to CI smoke-test route list

• Extends the curl-based smoke test loop to include the /legal/ path. This better matches sitemap-advertised coverage and prevents regressions for that route.

.github/workflows/site-checks.yml

@sonarqubecloud

Copy link
Copy Markdown

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@AlexanderWagnerDev
AlexanderWagnerDev merged commit dbdf3dd into agent/reach-and-seo Jul 18, 2026
3 checks passed
@AlexanderWagnerDev
AlexanderWagnerDev deleted the claude/review-fix-prs-i207bi branch July 18, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants