Remove trailing forward slash from void elements #922
Conversation
Is it really worth addressing here then? |
This is because these need to maintain compatibility with XHTML. You'll just have to ignore them. |
or submit upstream PR(s) to fix them? |
I think there's still something to be said for having consistency in our own code and it doesn't hurt to cut down on the validation noise a little bit.
I took a quick glance earlier and it seems like it would only require adding a configuration option, modifying the [gem's] template to use the option, and updating related tests. I'm definitely not the first person to want control over this behavior, so I was thinking about creating upstream PRs sometime in the future (unless I run into some kind of blocker in the process). |
samford
force-pushed
the
remove-slash-from-void-elements
branch
from
5377623 to
9a38652
Compare
|
To be honest I'm 👎🏻 on this change at least until it's fixed upstream too, sorry. |
|
Meh, personally I would prioritise template consistency over caring about what libraries do under the hood. I see this as defining a consistent style in our templates. Right now it's a dice roll what style we use, e.g. line 13 vs line 16 in newsletter.html, line 11 vs 25 in base.html |
|
Looking into this a bit, jekyll-seo-tag's template has self-closing tags and these are ~needed because Jekyll technically supports XHTML still (evidenced here and here, no official documentation alluding it beyond mentions in the changelog. I opened jekyll/jekyll-seo-tag#486 about it. |
|
@samford and I talked about this earlier... he's got a fork of jekyll-seo-tag with the necessary changes already made (and very similar to my napkin plan) so we'll see if the authors respond on the aforementioned issue. If they're receptive, then perhaps Sam can put up the change and we'll go from there. Much ado about slashes, haha. |
|
I'm currently seeing if I can reimplement my Jekyll plugin changes to use parameters (like you suggested in the issue) instead of |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
@samford I know there are a bajillion other far more important things… and I'm commenting to keep this PR open. |
|
Once conflicts are resolved: let's just merge this. I don't care enough either way. |
It's not necessary to self-close a void element in HTML5 and this is the cause of a lot of validation noise for us (unless restricted to only errors). We currently have void elements that use a trailing forward slash and some that don't, so this commit removes the forward slash from related elements. Unfortunately, our generated HTML will continue to include trailing forward slashes in void elements from the `jekyll-feed` and `jekyll-seo-tag` gems (where we can't control the output) but we can at least address this in our templates.
samford
force-pushed
the
remove-slash-from-void-elements
branch
from
9a38652 to
331d394
Compare
|
Thanks for quick fix here @samford 🙇🏻 |
|
Once I've finished working through some Outside of that, I still need to look into adding HTML validation (using |
It's not necessary to self-close a void element in HTML5 and this is the cause of a lot of validation noise for us (unless restricted to only errors). We currently have void elements that use a trailing forward slash and some that don't, so this PR removes the forward slash from related elements.
Unfortunately, our generated HTML will continue to include trailing forward slashes in void elements from the
jekyll-feedandjekyll-seo-taggems (where we can't control the output) but we can at least address this in our templates.This is based on #918 and #919 (including changes from both), so it will need to be merged after those PRs but it should cleanly rebase after #919.