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

Lowercase doctype. #2389

Closed
wants to merge 1 commit into from
Closed

Lowercase doctype. #2389

wants to merge 1 commit into from

Conversation

XhmikosR
Copy link
Contributor

It has the same effect but compresses slightly better.

@XhmikosR
Lowercase doctype.
75cc6f1 
It has the same effect but compresses slightly better.
@aymen94
Copy link
Member

aymen94 commented Aug 13, 2019

Thanks for contributing @XhmikosR, but HTML is Case insensitive, The DOCTYPEs can be either upper or lower case
for more info see: w3c HTML

@aymen94 aymen94 self-requested a review August 13, 2019 14:16
@XhmikosR
Copy link
Contributor Author

I'm aware of that hence this patch. There's no reason to use the uppercased doctype especially since the lowercase one should compress (slightly ofc) better.

@aymen94
Copy link
Member

aymen94 commented Aug 16, 2019

let's see what they think @nodejs/website

@3imed-jaberi
Copy link
Contributor

Thanks for your contribution @XhmikosR , I think there's no need to change it to lowercase.
I don't find a need to keep this PR open ....

@Trott
Copy link
Member

Trott commented Aug 16, 2019

If we have places where it is already lowercase, I'd be in favor of this for consistency although I'd want a lint rule to enforce it going forward.

@XhmikosR XhmikosR deleted the master-xmr-lowercase-doctype branch August 17, 2019 05:31
@XhmikosR
Copy link
Contributor Author

I don't mind not landing this, although I really don't see any gain from using an uppercase doctype.

For reference https://github.com/h5bp/html5-boilerplate/blob/master/dist/index.html#L1

And even google is using the lowercase doctype nowadays.

@Trott
Copy link
Member

Trott commented Aug 17, 2019

@nodejs/website Anyone feel like this is a generally-accepted best practice and therefore something we should do?

@XhmikosR
Copy link
Contributor Author

h5bp/html5-boilerplate#1522 (comment) about the small giz gain.

Personally, I say just lowercase it and be done with it. It's a leftover from the old days.

@silverwind
Copy link
Contributor

Is doctype even necessary any more in a modern browser? MDN says

its sole purpose is to prevent a browser from switching into so-called "quirks mode"

Given that no current browser except maybe Edge feature such a "quirks mode", is it even necessary?

@XhmikosR
Copy link
Contributor Author

I'd keep it for backwards compatibility. This patch proposes a simple change only.

@silverwind
Copy link
Contributor

Yeah I guess the spec saying "A DOCTYPE is a required preamble." should be honored. Anyways, I'm fine with lowercasing it to shave off a few bytes.

@XhmikosR XhmikosR restored the master-xmr-lowercase-doctype branch August 17, 2019 10:04
@XhmikosR
Copy link
Contributor Author

Can you reopen the PR or should I make a new one? I'm not sure if re-opening will work since I had deleted the branch.

@Trott Trott reopened this Aug 17, 2019
@pierreneter
Copy link
Contributor

DOCTYPE should be uppercase.

@XhmikosR
Copy link
Contributor Author

@pierreneter: the spec does not say that.

@pierreneter
Copy link
Contributor

So, why we need lowercase?

@XhmikosR
Copy link
Contributor Author

Because it has the same effect plus it compresses slightly better. See the previous links/discussion.

@pierreneter
Copy link
Contributor

Because it has the same effect plus it compresses slightly better. See the previous links/discussion.

I don't think so; so, I will give some ideas on why Uppercase is better.

  1. Note that if you don’t uppercase DOCTYPE in an XHTML document, the XML parser will return a syntax error.
    From: http://mathiasbynens.be/notes/xhtml5#doctype

  2. Most style guides and lint recommend DOCTYPE uppercase.
    From: https://google.github.io/styleguide/htmlcssguide.html#Document_Type
    So, if you use editor with any auto formatter tool, it will auto change to <!DOCTYPE html>, and this PR (Lowercase doctype. #2389) will be useless.

@XhmikosR
Copy link
Contributor Author

  1. We don't use XHTML so that's moot
  2. Check out google.com own source code

Anyway, I think this is discussed a lot more than it should be. Feel free to drop it or merge it.

@Trott
Copy link
Member

Trott commented Aug 17, 2019 •
edited
Loading

Because it has the same effect plus it compresses slightly better. See the previous links/discussion.

I don't think so; so, I will give some ideas on why Uppercase is better.

  1. Note that if you don’t uppercase DOCTYPE in an XHTML document, the XML parser will return a syntax error.
    From: http://mathiasbynens.be/notes/xhtml5#doctype

  2. Most style guides and lint recommend DOCTYPE uppercase.
    From: https://google.github.io/styleguide/htmlcssguide.html#Document_Type
    So, if you use editor with any auto formatter tool, it will auto change to <!DOCTYPE html>, and this PR (Lowercase doctype. #2389) will be useless.

XHTML compatibility (number 1 above) is a non-issue. We do not and should not conform to XHTML. Even the link provided to argue that doctype should be capitalized says, basically, don't bother in its very first sentence: "You know, I’ll always prefer HTML over XHTML because it’s much less verbose and I like to keep things simple."

As for the guide/lint thing (number 2 above): The linked style guide does not require capitalization. It uses capitalization in its example. It does not say "Don't use lowercase." Other sections have "not recommended" examples. That one does not. However, what that guide DOES say is to use HTML5 (where lowercase doctype is permitted) and to avoid XHTML.

AFAICT, the only persuasive reason for doing this is that it will (probably) shave a small number of bytes off some or all transfers. And the only persuasive reasons for not doing it are that it is unnecessary churn and that the bytes saved are insignificant.

For me, it's a coin toss. I can go either way.

@pierreneter
Copy link
Contributor

@Trott Ok, I agree with you.
Be honest, I prefer <!DOCTYPE html>, so, it's just about style and hobby.
But, if we must use <!doctype html>, we need to create PRs for all repositories which use <!DOCTYPE html> and convert it to <!doctype html> on Github?
If that's not necessary, why don't we keep the existing ones now? If this PR was merged, I will create PR with "Uppercase doctype." and discuss again?

@XhmikosR can you create PRs for all repositories which use <!DOCTYPE html> and convert it to <!doctype html> on Github? Let me know if you can. Because we can save 6B per data move time.
We can significantly reduce the data moving on the Internet if that is a large number of repositories changed and can save the resource for the Internet. Save the World. It can be. But need a lot of PRs. When creating PRs and comments, we move data. Maybe we can create a movement and call everyone to use <!doctype html> instead of <!DOCTYPE html> and add it to the spec? Do you agree?

@XhmikosR
Copy link
Contributor Author

What does it have to do what others do? I simply decided to contribute to a project which I use daily, here.

Like I said, we are discussing this a lot more than we should. For me it's standard to use lowercase doctype for years now. Others have a different opinion and that's OK.

@Trott
Copy link
Member

Trott commented Aug 17, 2019

I'm going to close this because it is indeed low-stakes.

@XhmikosR Note that there is a website-redesign initiative (no idea how far along they are), so for longevity of the change, you may want to see about implementing over there instead anyway. Not sure where they do their work these days (or if it's stalled? I hope not....) /ping @nodejs/website-redesign (I know they also manage the nodejs.dev repo/site but I think that's kind of a stop-gap measure and not a model for the new site.)

@Trott Trott closed this Aug 17, 2019
@XhmikosR XhmikosR deleted the master-xmr-lowercase-doctype branch August 17, 2019 16:27
@XhmikosR
Copy link
Contributor Author

@Trott for what it's worth, the core docs do use a lowercase doctype.

https://github.com/nodejs/node/blob/775048d54c6f190cbc8c0b55c0b53d2ba9d4d028/doc/template.html#L1

@XhmikosR XhmikosR restored the master-xmr-lowercase-doctype branch August 22, 2019 19:13
@XhmikosR
Copy link
Contributor Author

@Trott can you reopen this? IMO this totally makes sense especially given that a lowercase doctype is used in core.

@alexandrtovmach
Copy link
Contributor

I think it's just personal experience preferences... and we can't argue with something one for 100% sure, so we need to just vote. For me personally uppercase is more comfortable, but I had an experience with HTML pages optimization and used lowercase to reduce page size.

@nodejs/website-redesign @nodejs/website Let's vote on this post:

  • lowercase 👍
  • uppercase 👎

@XhmikosR
Copy link
Contributor Author

My point here is that this is how it is in core. https://github.com/nodejs/node/blob/master/doc/template.html#L1

@Trott
Copy link
Member

Trott commented Sep 29, 2019

@Trott can you reopen this?

The Reopen button is disabled, probably because the branch was deleted.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Sep 29, 2019 via email

@alexandrtovmach
Copy link
Contributor

Let's wait 48 hours for everyone able to vote

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Sep 29, 2019 via email

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

@aymen94 aymen94 aymen94 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@XhmikosR @aymen94 @3imed-jaberi @Trott @silverwind @pierreneter @alexandrtovmach