Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor: standardize the HTML, CSS and JSP in the webapps folder. #316

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.

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

Open
jbampton wants to merge 1 commit into apache:main
base: main
Choose a base branch
Loading
from jbampton:fix-html

Conversation

@jbampton
Copy link
Member

@jbampton jbampton commented Jul 4, 2020
edited
Loading

Use lowercase HTML tags and attributes.
Add quotes around attributes.
Standardize quotes.
Fix some HTML errors.
Use HTML5 and CSS.
Remove whitespace and blank lines.
Indent HTML and JSP.

Copy link

kkolinko commented Jul 5, 2020

error.html:

  1. Shouldn't it better be <form method="GET"?
  2. Options shall be closed with . The
    s there are out of place and ignored. (validator.w3.org complains)

Copy link
Member Author

jbampton commented Jul 6, 2020

Hey @kkolinko I removed the br tags and also closed the option tags now.

I also validated the 'error.html' by direct upload and it validates now.

https://validator.w3.org/#validate_by_input

@jbampton jbampton force-pushed the fix-html branch 7 times, most recently from 0b8fdf7 to e844277 Compare July 6, 2020 06:42
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta charset="UTF-8">
Copy link
Member

@martin-g martin-g Jul 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the auto-closing ? It doesn't do any harm. I, personally, prefer the style it was before.

Copy link
Member Author

@jbampton jbampton Jul 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In HTML5 you don't need the auto-closing.
I got the style from MDN.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta

Copy link
Member

@martin-g martin-g Jul 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but it does not do any harm either.
This change makes the diff bigger and harder to back-port to the other branches.
I prefer the XHTML style because it is easier to spot errors in the IDE/text editor without copying to W3C validator in the web.

stokito reacted with thumbs up emoji
Copy link
Member Author

error.html:

  1. Shouldn't it better be <form method="GET"?
  2. Options shall be closed with . The s there are out of place and ignored. (validator.w3.org complains)

MDN says -> Possible (case insensitive) values
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form

Screen Shot 2020年08月01日 at 5 31 33 am

@jbampton jbampton force-pushed the fix-html branch 8 times, most recently from bb5d215 to 735e437 Compare August 1, 2020 03:59
@jbampton jbampton changed the title (削除) Use lowercase HTML tags and attributes. Add quotes around attributes. (削除ここまで) (追記) Standardize the HTML, CSS and JSP in the webapps folder. (追記ここまで) Aug 1, 2020
@jbampton jbampton force-pushed the fix-html branch 2 times, most recently from 8c818fe to 1e7d050 Compare August 1, 2020 10:07
Use lowercase HTML tags and attributes.
Add quotes around attributes.
Fix some HTML errors.
Use HTML5 and CSS.
Indent HTML and JSP.
Remove whitespace.
Remove blank lines.
Standardize quotes.
@jbampton jbampton changed the title (削除) Standardize the HTML, CSS and JSP in the webapps folder. (削除ここまで) (追記) refactor: standardize the HTML, CSS and JSP in the webapps folder. (追記ここまで) Dec 17, 2020
@michael-o michael-o removed their request for review January 7, 2022 20:49
Copy link
Contributor

I'm a -1 on this change mostly because it's so large with so many changes at the same time. If this PR had been the accumulation of several commits e.g. (1) re-format whitespace everywhere with no other changes then (2) remove unnecessary auto-close elements then (3) standardize quoting, etc. then it would have been easier to review, spot things we didn't like, etc. But this is one giant change I'm unwilling to read through to decide if I like every single one of them so, no.

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

Reviewers

@martin-g martin-g martin-g requested changes

@michael-o michael-o michael-o approved these changes

+1 more reviewer

@stokito stokito stokito approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /