svelte icon indicating copy to clipboard operation
svelte copied to clipboard

fix: allow attributes on the title element

Open jschaf opened this issue 6 months ago • 6 comments

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

  • [x] 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
  • [x] Prefix your PR title with feat:, fix:, chore:, or docs:.
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] Ideally, include a test that fails without this PR but passes with it.
  • [x] 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.

  • [x] Run the tests with pnpm test and lint the project with pnpm lint

The tests pass, but pnpm lint failed with:

> [email protected] 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/[email protected]_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 avatar May 22 '25 00:05 jschaf

🦋 Changeset detected

Latest commit: 874011089b62bf98b91c53430297a0d4d0ccde24

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

changeset-bot[bot] avatar May 22 '25 00:05 changeset-bot[bot]

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

svelte-docs-bot[bot] avatar May 22 '25 00:05 svelte-docs-bot[bot]

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.

Rich-Harris avatar May 22 '25 02:05 Rich-Harris

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?

jschaf avatar May 22 '25 18:05 jschaf

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.

Rich-Harris avatar May 22 '25 18:05 Rich-Harris

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 avatar May 22 '25 23:05 jschaf