html does not validate for some views #662

Closed
hoboman313 opened this Issue Feb 14, 2012 · 6 comments

Comments

Projects
None yet
3 participants
@hoboman313

This is a general bug created to remind us that we have to validate our html.

Symptoms:

  • run rake test:functionals and observe the warnings get printed to screen

There seem to be a number of badly formated html elements such as the
tags ( should be
by xhtml standards ).

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Feb 14, 2012

Member

Is there a way to figure out which code snippet brings up those warnings?

Member

jerboaa commented Feb 14, 2012

Is there a way to figure out which code snippet brings up those warnings?

@hoboman313

This comment has been minimized.

Show comment
Hide comment
@hoboman313

hoboman313 Feb 14, 2012

The warnings are generated by "assert_select "div#errorExplanation"" on line 310 of the assignments_controller_test

The warnings are generated by "assert_select "div#errorExplanation"" on line 310 of the assignments_controller_test

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Feb 15, 2012

Member

Say there are more than one assert_select's in our testing code: E.g. 50. Would we be able to determine what view-template is causing those warnings? Or at least which assert_select causes HTML warnings? Probably not, but oh well...

Member

jerboaa commented Feb 15, 2012

Say there are more than one assert_select's in our testing code: E.g. 50. Would we be able to determine what view-template is causing those warnings? Or at least which assert_select causes HTML warnings? Probably not, but oh well...

@hoboman313

This comment has been minimized.

Show comment
Hide comment
@hoboman313

hoboman313 Feb 15, 2012

I think what we want is: https://github.com/realityforge/rails-assert-valid-asset
It should be able to do what you want.

I think what we want is: https://github.com/realityforge/rails-assert-valid-asset
It should be able to do what you want.

@hoboman313

This comment has been minimized.

Show comment
Hide comment
@hoboman313

hoboman313 Feb 25, 2012

Had a bit of time to look at this.

So far:

  • changed the doctype to xhtml strict
    • this has created A LOT of validation errors, as expected
    • the most common errors that I have seen are the use of "data-remote" property for some elements and putting elements where they are not allowed ( ex. cannot put a within a )

I have started using "assert_valid_markup" just because it already has a gem for it, otherwise it might be better to use "assert_valid_asset", which is very similar.

Two problems that I've encountered:

  1. Could not use the shortcut to check all the GET requests as some of these return ajax resonses, which have weird formatting.
  2. I don't think we can include or exclude specific tests because we use shoulda, so there aren't any test names to include/exclude ( as per the examples in the documentation given above ).

Imo, the best course of action is to go through all the functional tests and add the assert_valid_markup ( or equivalent ) to all the tests where we want to check the html validation. Even this will result in a lot of tests failing, so I am not sure whether it is worth it to create a separate branch for this issue or not.

Had a bit of time to look at this.

So far:

  • changed the doctype to xhtml strict
    • this has created A LOT of validation errors, as expected
    • the most common errors that I have seen are the use of "data-remote" property for some elements and putting elements where they are not allowed ( ex. cannot put a within a )

I have started using "assert_valid_markup" just because it already has a gem for it, otherwise it might be better to use "assert_valid_asset", which is very similar.

Two problems that I've encountered:

  1. Could not use the shortcut to check all the GET requests as some of these return ajax resonses, which have weird formatting.
  2. I don't think we can include or exclude specific tests because we use shoulda, so there aren't any test names to include/exclude ( as per the examples in the documentation given above ).

Imo, the best course of action is to go through all the functional tests and add the assert_valid_markup ( or equivalent ) to all the tests where we want to check the html validation. Even this will result in a lot of tests failing, so I am not sure whether it is worth it to create a separate branch for this issue or not.

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Feb 25, 2012

Member

I think we should change the doctype to HTML5. Rails generates many data-* attributes which aren't valid in XHTML 4 strict. As for validation, this will get hairy since I don't think there are good tools available to validate HTML5. validator.w3.org has only experimental HTML5 validation support. It's not (yet) an approved standard after all.

You should be able to use shoulda generated names to exclude tests, though. The test name is basically a long string. Put an assert false in any of the shoulda tests and it will tell you the test "name". It'll be something like " should ". Remember, shoulda is compatible with plain Test::Unit

It might be worth creating a branch for this, yes.

Member

jerboaa commented Feb 25, 2012

I think we should change the doctype to HTML5. Rails generates many data-* attributes which aren't valid in XHTML 4 strict. As for validation, this will get hairy since I don't think there are good tools available to validate HTML5. validator.w3.org has only experimental HTML5 validation support. It's not (yet) an approved standard after all.

You should be able to use shoulda generated names to exclude tests, though. The test name is basically a long string. Put an assert false in any of the shoulda tests and it will tell you the test "name". It'll be something like " should ". Remember, shoulda is compatible with plain Test::Unit

It might be worth creating a branch for this, yes.

hoboman313 pushed a commit to hoboman313/Markus that referenced this issue Feb 27, 2012

@reidka reidka closed this Jul 28, 2012

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