Skip to content

Don’t generate invalid <link rel="canonical"/> with missing href#3420

Merged
delucis merged 2 commits into
withastro:stack/invalid-canonicalfrom
andersk:invalid-canonical
Oct 22, 2025
Merged

Don’t generate invalid <link rel="canonical"/> with missing href#3420
delucis merged 2 commits into
withastro:stack/invalid-canonicalfrom
andersk:invalid-canonical

Conversation

@andersk

@andersk andersk commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Remove these invalid HTML tags from the output when context.site is unset:

  • <link rel="canonical"/> with missing href
  • <meta property="og:url"/> with missing content

@changeset-bot

changeset-bot Bot commented Sep 11, 2025 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 337c03a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify

netlify Bot commented Sep 11, 2025 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit 337c03a
🔍 Latest deploy log https://app.netlify.com/projects/astro-starlight/deploys/68f93e87389ee70008c28a16
😎 Deploy Preview https://deploy-preview-3420--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the 🌟 core Changes to Starlight’s main package label Sep 11, 2025
@astrobot-houston

astrobot-houston commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Hello! Thank you for opening your first PR to Starlight! ✨

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any issues you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🤩

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

@andersk andersk force-pushed the invalid-canonical branch 2 times, most recently from 4822238 to 40e2fd2 Compare September 11, 2025 22:19
trueberryless
trueberryless reviewed Sep 12, 2025
View reviewed changes

@trueberryless trueberryless left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this and all the other awesome PRs.

These changes look good to me. While I couldn't find a resource stating that those tags are invalid without the content attribute, the semantic meaning of those tags is empty.

Here are the resources I found that only say how the tag is intended:

Comment thread .changeset/quick-apes-grab.md Outdated
Comment thread packages/starlight/utils/head.ts Outdated
@andersk
Don’t generate invalid <link rel="canonical"/> with missing href
7319a41 
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk

andersk commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

Adjusted as suggested, thanks.

The specification for <link> href says:

One or both of the href or imagesrcset attributes must be present.

For <meta> content, since OpenGraph is based on RDFa, here’s the relevant specification:

If the RDFa @property attribute is present on the meta element, neither the @name, @http-equiv, nor @charset attributes are required and the @content attribute MUST be specified.

delucis
delucis requested changes Sep 13, 2025
View reviewed changes

@delucis delucis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to write some tests for this change to make sure we don’t accidentally regress. These can probably go in packages/starlight/__tests__/head/head.test.ts — we might need to update the getRouteDataTestContext and getTestHead helpers to allow for testing with site not set.

@delucis delucis changed the base branch from main to stack/invalid-canonical October 22, 2025 20:29
@delucis

delucis commented Oct 22, 2025

Copy link
Copy Markdown
Member

Thanks again for the PR @andersk!

@erbierc has kindly offered to take over writing tests for this, so I’m going to merge this into an intermediate branch — https://github.com/withastro/starlight/tree/stack/invalid-canonical — so we can collaborate there before merging into main.

@delucis delucis merged commit 2dbc627 into withastro:stack/invalid-canonical Oct 22, 2025
16 checks passed
@delucis delucis mentioned this pull request Oct 22, 2025
Merged
@erbierc erbierc mentioned this pull request Oct 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delucis delucis delucis requested changes
+1 more reviewer
@trueberryless trueberryless trueberryless left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🌟 core Changes to Starlight’s main package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andersk @astrobot-houston @delucis @trueberryless