search

elixirschool / school_house

The new era of Elixir School now powered by @phoenixframework
https://elixirschool.com/
Apache License 2.0
154 stars 49 forks source link

HTML issues with meta tags #94

Closed jaimeiniesta closed 3 years ago

jaimeiniesta commented 3 years ago

The most common HTML issues we have are related with <meta> tags generated by Phoenix:

https://rocketvalidator.com/s/0bdee490-cff7-495a-95a3-40211cd2b5cd/i?search=meta

There are all sort of issues but in the end it's just one line in the layout:

https://github.com/elixirschool/school_house/blob/master/lib/school_house_web/templates/layout/root.html.leex#L6

<%= csrf_meta_tag() %>

This function comes from PhoenixHTML, and is documented here:

https://hexdocs.pm/phoenix_html/Phoenix.HTML.Tag.html#csrf_meta_tag/0

Now, trouble with this meta tag is that the W3C HTML Validator says it's malformed. You can see by yourself on https://validator.w3.org/nu/#textarea and checking this example HTML:

<!DOCTYPE html>
<html lang="">
<head>
<title>Test</title>
<meta charset="UTF-8" content="ASonEyRBAAJaA2srAAZrZlkhYmYeGHUFHdsgU5uN9iYdN_3_tRV_njE_" csrf-param="_csrf_token" method-param="_method" name="csrf-token">
</head>
<body>
<p></p>
</body>
</html>

This raises 4 errors:

I see those options:

1.- Muting all those issues

We can just accept that this is generated by Phoenix so we won't fix those issues, and just mute them.

2.- Try to generate valid meta tags

What I do on the RV site is change it to:

<meta charset="UTF-8">
<meta content="<%= get_csrf_token() %>" name="csrf-token">

which fixes the issues, it moves the charset declaration to its own tag as expected, and simplifies the one for the CSRF token. But we would be missing the csrf-param="_csrf_token" method-param="_method" which I'm not sure how they're generated.

What do you think?

jaimeiniesta commented 3 years ago

Here's how PhoenixHtml generates those attributes, they're static:

https://github.com/phoenixframework/phoenix_html/blob/master/lib/phoenix_html/tag.ex#L13-L14

I think that the generated meta tag could be decomposed in 3:

<!-- Generated by phoenix_html -->
<meta charset="UTF-8" content="cSAFLjcnRyE5M3UENw4eWmEYBHcEK0oT8nQZFS2mZYGKyWFcLk0NtYzI" csrf-param="_csrf_token" method-param="_method" name="csrf-token">

<!-- Could be split in 3 -->
<meta charset="UTF-8">
<meta name="csrf-token" content="cSAFLjcnRyE5M3UENw4eWmEYBHcEK0oT8nQZFS2mZYGKyWFcLk0NtYzI">
<meta csrf-param="_csrf_token" method-param="_method">

Of those 3, the first 2 ones are valid but the last one is not. I can't think of a way to make that valid. So unless you have a better approach, or feel safe removing that one, we should just mute those issues.

jaimeiniesta commented 3 years ago

Related:

https://github.com/phoenixframework/phoenix_html/issues/294

kinson commented 3 years ago

1) Thank you for going down this rabbit hole, I never thought I'd spend Sunday morning learning about acceptable <meta /> tag contents 😆 2) Is José human? He reopened the year old issue and put in a change for it within an hour of your comment! Incredible 👏

It looks like we can omit the last tag (csrf-param="_csrf_token" method-param="_method") for now then 🎉 and go back to the built in Phoenix helper (<%= csrf_meta_tag() %>) once the next version of Phoenix is out, right?

jaimeiniesta commented 3 years ago

Yes, José is awesome :)

It was fun to find my past self asking for this same issue one year ago 😆

I'll change that part to this until they release the next version:

<meta charset="UTF-8">
<meta content="<%= get_csrf_token() %>" name="csrf-token">