-
-
Notifications
You must be signed in to change notification settings - Fork 810
Don’t generate invalid <link rel="canonical"/>
with missing href
#3420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 7319a41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Hello! Thank you for opening your first PR to Starlight! ✨ Here’s what will happen next:
|
4822238
to
40e2fd2
Compare
There was a problem hiding this comment.
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:
- https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/meta/name
- https://ogp.me/
- https://www.w3.org/TR/rdfa-primer/#using-the-content-attribute
While there is no problem using
link
in RDFa Lite (which uses thehref
attribute, i.e., can be used to define "flavored" links), the fact that, in a conforming HTML file, themeta
element may have no text content means that the only way of using the header for such statements is to use thecontent
attribute.
Signed-off-by: Anders Kaseorg <[email protected]>
40e2fd2
to
7319a41
Compare
Adjusted as suggested, thanks. The specification for
For
|
There was a problem hiding this comment.
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.
Remove these invalid HTML tags from the output when
context.site
is unset:<link rel="canonical"/>
with missinghref
<meta property="og:url"/>
with missingcontent