Skip to content

fix: allow attributes on the title element #15983

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: allow attributes on the title element #15983

wants to merge 2 commits into from

Conversation

jschaf
Copy link

@jschaf jschaf commented May 22, 2025

The spec states the title element supports global attributes. https://html.spec.whatwg.org/#the-title-element

Reverts the title attribute validation introduced in #1721.

Fixes #5198.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

The test plan is to re-use packages/svelte/tests/validator/samples/title-no-attributes. By removing errors.json, we test that svelte compiles the title element with attributes.

  • Run the tests with pnpm test and lint the project with pnpm lint

The tests pass, but pnpm lint failed with:

> svelte-monorepo@0.0.1 lint /opt/p/svelte
> eslint && prettier --check .


Oops! Something went wrong! :(

ESLint: 9.9.1

Error: Cannot find module '/opt/p/svelte/node_modules/.pnpm/svelte-eslint-parser@0.43.0_svelte@packages+svelte/node_modules/svelte/compiler/index.js'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1441:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1430:15)

@jschaf
fix: allow attributes on the title element
5b73606 
The spec states the title element supports global attributes.
https://html.spec.whatwg.org/#the-title-element

Reverts the title attribute validation introduced in sveltejs#1721.

Fixes sveltejs#5198.
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 22, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 8740110

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

This PR includes changesets to release 1 package
Name Type
svelte 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

@svelte-docs-bot
Copy link

Preview: https://svelte-dev-git-preview-svelte-15983-svelte.vercel.app/

@jschaf jschaf mentioned this pull request May 22, 2025
Open
@Rich-Harris
Copy link
Member

Thank you. The aria-live thing seems like a good reason to have this but it's not quite as straightforward as this PR — you've removed the validation, but the <title> still won't have any attributes.

When you create a <title> in a component, Svelte doesn't actually create a <title> element and append it to the head, it instead sets document.title, because you're only supposed to have one. We could probably change that (each new <title> inside a <svelte:head> would be prepended to the <head> instead, and removed when the component was unmounted — browsers will do the right thing in that case) but we'd need to change the implementation in addition to removing the validation.

@jschaf
Copy link
Author

jschaf commented May 22, 2025

Thank you. The aria-live thing seems like a good reason to have this but it's not quite as straightforward as this PR — you've removed the validation, but the <title> still won't have any attributes.

Ah, I see. I'll take a crack at rendering the attributes. Should I limit attributes to a subset?

@Rich-Harris
Copy link
Member

I don't think there's any reason to limit it, it's extra work to prevent something that someone might find useful in ways we don't anticipate.

I think the most interesting challenge here will be hydration, since in the (admittedly rare!) case of duplicate elements they need to be rendered in reverse order, so that 'later' <title> elements (e.g. in a +page.svelte) take precedence over 'earlier' ones (e.g. a fallback in a +layout.svelte). Quite why browsers work that way is a mystery, of course.

@jschaf
WIP: attempt to add render title attributes
8740110 
SSR is pretty straight-forward. The client side is more difficult since
the TitleElement doesn't have a node_id.
@jschaf
Copy link
Author

jschaf commented May 22, 2025

I added SSR title tag attributes by cribbing code from RegularElement in the latest commit. Client-side rendering is trickier, and I could use some pointers.

The TitleElement doesn't have a reference to the HTML element because we didn't need one before. To call build_set_attributes, I need a node_id, from context.state.node.

The code for adding attributes is inlined in the client RegularElement. I extracted the smallest chunk of code into TitleElement to make it work.

  • How should I go about getting the node_id for a TitleElement?

  • Should I use RegularElement to render TitleElement?

  • I'm still using the assignment to document.title to set the title value. Should I keep that approach or append the title element to the head element
    The challenge with appending is that spec states multiple title tags are not allowed: The title tag may be used in "head element containing no other title elements."

@jschaf jschaf marked this pull request as draft May 22, 2025 23:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow attributes on the <title> element
2 participants
@jschaf @Rich-Harris