search

jekyll / jekyll-seo-tag

A Jekyll plugin to add metadata tags for search engines and social networks to better index and display your site's content.
https://jekyll.github.io/jekyll-seo-tag
MIT License
1.65k stars 289 forks source link

Remove trailing slash from closing tag #480

Closed potaito closed 4 months ago

potaito commented 1 year ago

According to https://validator.w3.org trailing slashes like these should not be used:

"Trailing slash on void elements has no effect and interacts badly with unquoted attribute values"

It's just an INFO message, but doesn't hurt to adhere to the recommendation I think. Below is an example Screenshot of the results I get when validating a page generated with jekyll and jekyll-seo-tag:

screenshot

Test

I tested the changes locally and re-uploaded the generated HTML file to the validator. The w3 validator was happy :)

sasadangelo commented 1 year ago

Hi potaito, I need that this feature is included into the Jekyll Tags code. I have my website on GitHUB, what I can do to have this fix in the meanwhile it enter in the official code? I think this fix is not disruptive and easy to include. Why hasn't it been included yet?

potaito commented 1 year ago

We can kindly ask @ashmaroli and @mattr- to take a look. They probably just didn't get around to it yet. I'm not a maintainer, so I can't do more than open a PR.

sasadangelo commented 1 year ago

Ok thank you!

ashmaroli commented 1 year ago

Hello everyone, Unfortunately, the proposed change(s) are not as benign as it seems. HTML is lax in terms of an element's closing tag. But XHTML isn't. To my knowledge, XHTML requires an element to be explicitly closed, including void elements.

Since, removing the slash from the closing tags will be a breaking change for users with XHTML templates, I am not willing to merge this right away. I am however, open to accept this when there is significant evidence to favor the move.

sasadangelo commented 1 year ago

Hi ashmaroli,

Thanks you for the reply. You're right, this change isn't XHTML compatible. However, can you tell me an example of "breaking change for users with XHTML templates"? My understanding is that this plugin is for Jekyll that is built to create static website. Static websites are visible thorugh browsers. All browsers support HTML 5 and previous versions that, according to the W3C standard uses void elements like meta, link, img that don't expect content inside so they don't need closing tag. As far as I know latest XHTML version was 2.0 which was under development for several years but was never completed or released. This version of XHTML was eventually abandoned in favor of a new approach called HTML5.

I don't want convince anyone, I just want to understand what's wrong with my reasoning.

ashmaroli commented 1 year ago

can you tell me an example of "breaking change for users with XHTML templates"? My understanding is that this plugin is for Jekyll that is built to create static website..

Say, I have a site configured with permalink: "/:categories/:title.xhtml" with the base layout with:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
  {% seo %}
</head>
<body>
{{ content }}
</body>
</html>

It wouldn't be wise for {% seo %} to render non-XHTML-compliant markup in this case, would it?

sasadangelo commented 1 year ago

Just a question: are there people out there wrinting web pages with these specification?

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">

Why not HTML 5? However, I got your point. To be honest I don't agree too much but it is respectable. In this case, to be honest, add SEO tags like the one your plugn include it's not hard to do, people like me are forced to move to its own impementation. Thank you for your reply.

sasadangelo commented 1 year ago

At this point, why not an option to remove the trailing slash on void elements (meta, link, img, etc.) for HTML standard?

ashmaroli commented 1 year ago

are there people out there wrinting web pages with these specification?

I personally doubt there are any, in this day and age. But as a maintainer, I cannot make such assumptions. What I can do is to take a stand to explicitly drop support for XHTML compatibility and release with a major version bump. However, GitHub Pages will not update their stack to preserve backwards-compatibility at their end.

ashmaroli commented 1 year ago

why not an option to remove the trailing slash on void elements (meta, link, img, etc.) for HTML standard?

Because it is sub-optimal. The options are to either maintain two separate templates or introduce numerous {% if seo_tag.xhtml_mode %}..., both of which are not worth the effort.

potaito commented 1 year ago

Thanks for the explanation @ashmaroli . I have not considered XHTML when I proposed this.

sasadangelo commented 1 year ago

Ok thank you @ashmaroli for explanation. Your tool is generic and make sense support both the standards.

sasadangelo commented 1 year ago

Hi @ashmaroli,

You already explained that this fix cannot be done on your side. I agree with you. I am not a Jekyll expert, especially in Plugin write. Is there any code I can write on my side just to override your implementation for meta and links tags?

ashmaroli commented 1 year ago

@sasadangelo Out-of-the-box, there is no support for a custom template. However, there is a convoluted workaround that involves using GitHub Actions to build and deploy your Jekyll site(s):

  1. Set up GitHub Actions to build and deploy site.
  2. Replace gem github-pages with individual constituent dependencies in the Gemfile. (Create a new Gemfile via bundle init, if it doesn't exist.)
  3. Modify gem "jekyll-seo-tag" listing in Gemfile to point to this pull request (unaffected by branch-deletion): 
    - gem 'jekyll-seo-tag', '...'
+ gem 'jekyll-seo-tag', github: 'jekyll/jekyll-seo-tag', ref: 'refs/pull/480/head'

Eventually, if you feel the need to make more changes to the plugin, you can fork this repository, commit all desired changes to a branch and edit the Gemfile to point to that branch instead.

sasadangelo commented 1 year ago

Hi ashmaroli,

Thank you for your reply. Yes this is a thing I thought. My question is if it is possible add a piece of ruby code on plugins folder just to override meta and link tags.

ashmaroli commented 1 year ago

Unfortunately @sasadangelo, that is not an option since the change you require is in a Liquid template.

Mohamed3nan commented 1 year ago

@sasadangelo Out-of-the-box, there is no support for a custom template. However, there is a convoluted workaround that involves using GitHub Actions to build and deploy your Jekyll site(s):

  1. Set up GitHub Actions to build and deploy site.
  2. Replace gem github-pages with individual constituent dependencies in the Gemfile. (Create a new Gemfile via bundle init, if it doesn't exist.)
  3. Modify gem "jekyll-seo-tag" listing in Gemfile to point to this pull request (unaffected by branch-deletion): 
    - gem 'jekyll-seo-tag', '...'
+ gem 'jekyll-seo-tag', github: 'jekyll/jekyll-seo-tag', ref: 'refs/pull/480/head'

Eventually, if you feel the need to make more changes to the plugin, you can fork this repository, commit all desired changes to a branch and edit the Gemfile to point to that branch instead.

This worked in my case without any conflicts Thanks image