Skip to content
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.

Sign up for GitHub

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

Jump to bottom

When charset is set inside the SEO component, it's placed after the title tag #91

Open
ttmc opened this issue Feb 24, 2024 · 8 comments
Open

When charset is set inside the SEO component, it's placed after the title tag #91

ttmc opened this issue Feb 24, 2024 · 8 comments
Assignees
@jonasmerlin
Labels
bug Something isn't working help wanted Extra attention is needed waiting for feedback

Comments

@ttmc
Copy link

ttmc commented Feb 24, 2024

These days, a typical HTML5 file will begin something like:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    ...
    <title>Title comes later</title>

The meta charset tag is almost always the first tag inside the head tag, because the HTML5 specification suggests placing the charset declaration within the first 1024 bytes of the document.

Placing the charset declaration first clarifies the character encoding for the entire document, including the title, ensuring proper interpretation of characters. While modern browsers are robust, in rare cases, placing the title before the charset might lead to rendering issues, especially if the title includes non-ASCII characters.

The SEO component provided by astro-seo lets one set the value of charset. If one does that, the generated HTML has the title tag before the charset declaration. That is an issue.

Thoughts on How to Fix this Issue

The first thing you might be tempted to do is put the meta charset tag first, before the title tag. The problem with that is that there might be lots of other tags before the SEO component, inside the head tag. The SEO component has no control over those tags. They're put there by the user of the SEO component, not by the SEO component itself.

The final generated HTML should have the charset declaration first (inside the head tag), yet the SEO component can't do that.

Moreover, the charset declaration isn't really an SEO thing anyhow. It's more like the meta viewport tag.

Therefore, my conclusion is that the SEO component provided by astro-seo should be changed to no longer accept a value for charset, and it should no longer try to set it. Anyone who tries to use the SEO component to set charset should be told it's being deprecated and they should just set it manually, first thing inside the head tag. Then in the future, the charset prop could be removed from the SEO component completely.
@jonasmerlin
Copy link
Owner

@ttmc Thank you for this great report and thorough engagement with the issue. This is greatly appreciated and I think I agree that the best move here might be to deprecate the attribute.

Another possibility would be to run some JS that would always put the tag as the first child of the head. What's your opinion on that?

@jonasmerlin jonasmerlin self-assigned this Feb 25, 2024
@jonasmerlin jonasmerlin added bug Something isn't working help wanted Extra attention is needed waiting for feedback labels Feb 25, 2024
@jonasmerlin jonasmerlin mentioned this issue Feb 25, 2024
Open
@ttmc
Copy link
Author

ttmc commented Feb 25, 2024

@jonasmerlin You wrote:

Another possibility would be to run some JS that would always put the tag as the first child of the head. What's your opinion on that?

Yes, that might be a better solution. I was reluctant to suggest it because it's a bit weird for a component to be reaching outside its boundaries and adding a tag elsewhere in the document. Maybe also check if there's already a charset declaration before doing anything...

@ttmc
Copy link
Author

ttmc commented Feb 26, 2024 •
edited
Loading

If you do decide to use JavaScript to check for an existing charset declaration, and to inject one at the top of the head if necessary, you will have to be very careful to make sure that JavaScript code runs at the right time.

If you run it at build time (inside the --- fences at the top of an .astro component file), then what HTML "document" is it going to inspect / inject-into? If you put it in the <script> tag of the .astro component file, then that will get injected at the end of the head tag and the user agent / browser won't execute it until it's too late: it will already be done reading/executing the part of the head where the charset declaration should be.

The JavaScript should run after-building but before-deploying the generated HTML documents. Good news: Astro integrations can do that, see https://docs.astro.build/en/reference/integrations-reference ... I've never done that but I guess you can look at existing integrations like @astrojs/sitemap to see how they work.

@jonasmerlin
Copy link
Owner

@ttmc Thank you for your insights on this, these are very helpful! I wanted to sit down to implement this today, but I don't think I have enough time to turn this into an integration and see if this works. Might I ask if you would be willing to give this a try and then do a PR for this?

@ttmc
Copy link
Author

ttmc commented Mar 3, 2024

I was thinking more about what it would take to insert the charset declaration after-the-fact, into a fully-built website (i.e. all generated HTML files) and realized that wow, this is nuts, way beyond what the user of an SEO component would expect it to do.

Maybe the easiest solution is the following:

  1. Change the README.md file in a few small ways. Change "In any of your Astro pages, import Astro SEO and then use the component inside the <head> section of your HTML:" to "In any of your Astro pages, import Astro SEO and then use the component first thing inside the <head> section of your HTML:"
  2. In the sample HTML code, insert an HTML comment first thing after the <head> tag like so:
---
import { SEO } from "astro-seo";
---

<html lang="en">
  <head>
    <!-- (DELETE THIS COMMENT) Please note that the SEO component will insert
         a meta charset declaration first (with default value UTF-8) and
         a meta viewport declaration second; you don't need to do that yourself. -->
    <SEO
      title="A Very Descriptive Title"
      ...

(Also, make sure the default charset is "UTF-8". Also I would have the default meta viewport declaration as <meta name="viewport" content="width=device-width, initial-scale=1" /> )

I think these changes are much more straightforward to make, and they don't cause the SEO component to do anything surprising.

@millette
Copy link

I don't really like the idea of using client-side js to place charset ahead of the rest. Maybe things changed, but I remember browsers read a page until they hit the charset and might reload (not visible to the user) after that.

I'm starting to think the best option would be NOT to support charset (warn if used) and let the user insert the meta charset where necessary.

@Mansi1
Copy link
Contributor

Mansi1 commented Apr 4, 2024

Regarding the SEO library for Astro, while the charset and viewport metadata is crucial for rendering, it's not directly tied to SEO. Let's prioritize optimizing SEO elements, including interesting meta tags and maintain best practices for usability.

@paulrudy paulrudy mentioned this issue Oct 10, 2024
Closed
@ttmc
Copy link
Author

ttmc commented Nov 20, 2024

I think I found a nice solution: just tell users of astro-seo to also install and use astro-capo.

"astro-capo is a component that automatically optimizes the order of elements in your <head>..."

Among other things, astro-capo will make sure the meta charset declaration comes first in the final <head>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonasmerlin jonasmerlin

Labels
bug Something isn't working help wanted Extra attention is needed waiting for feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@millette @ttmc @jonasmerlin @Mansi1