[#675] Added SEO and head assertion steps to 'MetatagTrait'.#688
Merged
Conversation
Added canonical URL, indexability, robots directive, hreflang and social-tag assertions on top of the existing generic meta-tag assertion, reusing its lookup internals: - Canonical URL presence, absence and value. - Indexability via the robots meta tag and the 'X-Robots-Tag' response header, plus robots directive inclusion checks. - hreflang alternate validity (self-reference and language codes) and reciprocal return links fetched out of band. - Open Graph and Twitter Card minimum-set and explicit-set assertions. Added a fixture module route that emits an 'X-Robots-Tag' header and BDD coverage for every new step. Claude-Session: https://claude.ai/code/session_012WhCKmn7GbhNCQgfXmWqXC
…rage. Matched the full indexability failure messages and added failure scenarios for the table-driven Open Graph and Twitter Card 'should exist' steps. Claude-Session: https://claude.ai/code/session_012WhCKmn7GbhNCQgfXmWqXC
WalkthroughMetatagTrait now includes SEO/head assertions for canonical URLs, robots/indexability, hreflang, and Open Graph/Twitter Card tags. The PR also adds supporting fixtures, Drupal test endpoints, Behat scenarios, and updated step documentation. ChangesSEO metatag assertions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Scenario as Behat scenario
participant Trait as MetatagTrait
participant curl as curl
participant AltPage as alternate page
Scenario->>Trait: metatagAssertHreflangReciprocal()
Trait->>Trait: collect alternates and resolve URLs
Trait->>curl: fetch alternate URL
curl->>AltPage: HTTP GET
AltPage-->>curl: HTML response
curl-->>Trait: fetched HTML
Trait->>Trait: verify return link to current URL
Trait-->>Scenario: pass or fail
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MetatagTrait.php`:
- Around line 319-320: The reciprocal hreflang check is resolving link hrefs
against the Mink base URL instead of the fetched alternate page URL. Update
metatagHtmlLinksBackTo() so relative links are resolved using the alternate page
URL returned by metatagFetchUrl() (or pass that URL through from the callers in
MetatagTrait), ensuring nested paths and cross-domain alternates are checked
against the correct base. Apply the same fix in the other hreflang validation
call sites mentioned by metatagHtmlLinksBackTo(), metatagFetchUrl(), and the
related hreflang verification logic.
- Around line 175-178: The canonical check in
MetatagTrait::metatagGetCanonicalHref handling is only rejecting non-empty href
values, so an empty href incorrectly passes as “not set.” Update the
canonical-existence logic to treat any present canonical link element as present
even when href is empty, and keep throwing the exception from the surrounding
MetatagTrait validation when a canonical tag exists regardless of its href
contents.
- Around line 574-577: Save the current libxml internal error setting before
calling libxml_use_internal_errors(TRUE) in MetatagTrait’s HTML parsing flow,
then restore that saved value in a finally block after DOMDocument::loadHTML and
libxml_clear_errors run. Update the same code path so the parser state change is
scoped locally and later callers do not inherit the suppressed error mode.
- Around line 303-319: metatagAssertHreflangReciprocal() is using alternate href
values directly, so empty or invalid links can be resolved and fetched before
validation. Update the reciprocal check to validate each alternate with
metatagAssertHreflangValid() (or equivalent validation flow) before calling
metatagResolveUrl() or metatagFetchUrl(), and skip or fail fast on invalid
alternates so the existing clear validation error is surfaced instead of
treating empty hrefs as the base URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f5fa2117-7e4b-48a3-8062-f2a0aaf6ed30
📒 Files selected for processing (17)
README.mdSTEPS.mdsrc/MetatagTrait.phptests/behat/features/metatag.featuretests/behat/fixtures/metatags_hreflang_404.htmltests/behat/fixtures/metatags_hreflang_de.htmltests/behat/fixtures/metatags_hreflang_emptyhref.htmltests/behat/fixtures/metatags_hreflang_en.htmltests/behat/fixtures/metatags_hreflang_invalid.htmltests/behat/fixtures/metatags_hreflang_noreturn.htmltests/behat/fixtures/metatags_hreflang_noself.htmltests/behat/fixtures/metatags_noindex.htmltests/behat/fixtures/metatags_seo.htmltests/behat/fixtures_drupal/d10/web/modules/custom/mysite_core/mysite_core.routing.ymltests/behat/fixtures_drupal/d10/web/modules/custom/mysite_core/src/Controller/TestContent.phptests/behat/fixtures_drupal/d11/web/modules/custom/mysite_core/mysite_core.routing.ymltests/behat/fixtures_drupal/d11/web/modules/custom/mysite_core/src/Controller/TestContent.php
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #688 +/- ##
==========================================
+ Coverage 97.19% 97.27% +0.07%
==========================================
Files 52 52
Lines 4283 4404 +121
==========================================
+ Hits 4163 4284 +121
Misses 120 120 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ing. - Failed 'the canonical URL should not exist' when a canonical link element is present with an empty href. - Validated hreflang alternates before reciprocal fetches so an empty href no longer resolves to the base URL. - Resolved a fetched alternate page's relative links against that page's own origin instead of the Mink base URL. - Saved and restored the libxml internal-error mode around out-of-band HTML parsing. Claude-Session: https://claude.ai/code/session_012WhCKmn7GbhNCQgfXmWqXC
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MetatagTrait.php`:
- Around line 613-626: Handle document-relative hreflang/canonical URLs in
metatagResolveUrl() or document that only root-relative links are supported. The
current origin-only resolution in metatagResolveUrl() strips the fetched page
path, so values like about.html or ../en/about are resolved against the site
origin instead of the current document URL, which breaks
metatagHtmlLinksBackTo() and the self-reference check. Update the resolution
logic to use the fetched page URL when provided (or otherwise resolve relative
to the document context), and keep the special handling for absolute and
root-relative URLs intact.
In `@tests/behat/features/metatag.feature`:
- Around line 206-219: Treat empty canonical href values as absent in
metatagAssertCanonicalNotExists() so it matches metatagAssertCanonicalExists()
and metatagAssertCanonicalEquals(). Update the assertion logic in the
MetatagTrait step implementation to consider both NULL and empty string as
missing, and adjust the behat scenario for “Then the canonical URL should not
exist” to expect success when the canonical link has href="" instead of a
failure message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c23c9a4e-0225-4095-aa99-71516411119c
📒 Files selected for processing (3)
src/MetatagTrait.phptests/behat/features/metatag.featuretests/behat/fixtures/metatags_canonical_empty.html
…solution. - Treated an empty canonical href as absent in 'the canonical URL should not exist', consistent with the presence and value assertions. - Documented that 'metatagResolveUrl()' supports absolute and root-relative URLs and resolves document-relative URLs against the origin. Claude-Session: https://claude.ai/code/session_012WhCKmn7GbhNCQgfXmWqXC
AlexSkrypnyk
added
the
Needs review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #675
Summary
MetatagTraitpreviously only offered generic<meta>tag presence, content, and no-HTML assertions, leaving common SEO/head checks to be hand-rolled in consumer projects.This PR adds higher-level SEO and head assertion steps on top of the existing generic assertions: canonical URL presence/absence/value, indexability (robots meta tag and
X-Robots-Tagheader) and robots-directive checks, hreflang alternate validity (self-reference and language-code format) and reciprocal return-link checks, and Open Graph / Twitter Card minimum-set and explicit-table assertions.All new steps reuse the trait's existing meta-lookup internals and
HelperTrait, and the trait reaches 100% line coverage.Changes
Canonical URL -
the canonical URL should be :url,the canonical URL should exist, andthe canonical URL should not exist, comparing the<link rel="canonical">href resolved to absolute form against the Mink base URL. The three steps treat an empty href consistently as "no canonical URL", soshould existfails andshould not existpasses for a present-but-empty canonical.Indexability and robots -
the page should be indexable/should not be indexable(checks both the robots meta tag and theX-Robots-Tagresponse header for anoindex/nonedirective), plusthe meta robots should include :directive/should not include :directivefor direct directive assertions with whole-token, case-insensitive matching.Hreflang -
the hreflang alternates should be validchecks that at least one alternate exists, every hreflang value is a well-formed language code (orx-default), and a self-referencing alternate for the current URL is present.the hreflang alternates should have reciprocal return linksfirst runs that validation, then fetches each non-self, non-x-defaultalternate out of band via cURL and asserts it links back to the current URL, resolving the fetched page's relative links against that page's own origin.Open Graph and Twitter Card -
the Open Graph tags should be valid/the following Open Graph tags should exist:andthe Twitter Card tags should be valid/the following Twitter Card tags should exist:, asserting a default or caller-supplied set of properties is present and non-empty. The default required sets are exposed as overridable protected methods.Internals -
metatagAssertNoHtml()now delegates to a new sharedmetatagFindMeta()helper instead of duplicating the XPath lookup; the trait pulls inHelperTraitfor comma-separated directive parsing (helperSplitCommaSeparated()). Out-of-band HTML parsing saves and restores the libxml internal-error mode.Fixture module - adds a
/mysite_core/test-robots-headerroute (with avaluequery parameter controlling the emitted header) to themysite_corecustom module in both the d10 and d11 fixture sites, so theX-Robots-Tagheader path can be tested against a real HTTP response.Fixtures - 10 new HTML fixtures under
tests/behat/fixtures/: a fully-populated SEO page, anoindexpage, an empty-canonical page, and hreflang variants (self-referencing pair, missing self-reference, missing return link, invalid language code, empty href, 404 target).BDD coverage -
tests/behat/features/metatag.featuregrows from 8 to 38 scenarios: positive-path assertions for every new step plus@trait:MetatagTraitfailure-message scenarios (run as Behat subprocesses) verifying each exception message, bringingMetatagTraitto 100% line coverage.Docs - regenerated
STEPS.mdwith the new step documentation and updated theMetatagTraitone-line summary inREADME.md.Before / After