Skip to content

Tidy does not complain about valign in <tr>, <th> or <td> #711

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
Closed
petdance opened this issue Apr 3, 2018 · 5 comments
Closed

Tidy does not complain about valign in <tr>, <th> or <td> #711

petdance opened this issue Apr 3, 2018 · 5 comments
Labels
Technical Discussion
Milestone
5.7

Comments

@petdance
Copy link
Contributor

petdance commented Apr 3, 2018

This file gives no errors:

<!DOCTYPE html>
<html>
    <head>
        <title>Blah</title>
    </head>
    <body>
        <table>
            <tr valign="middle">
                <th valign="middle">
                    heading
                </th>
                <td valign="top">
                    cell
                </td>
            </tr>
        </table>
    </body>
</html>

What I see shows that valign is no longer valid in HTML5
@geoffmcl
Copy link
Contributor

geoffmcl commented Apr 4, 2018

@petdance thank you for the issue...

Agree absolutely tidy running in html5 should for sure bleat about these obsolete attributes. And of course not if a legacy html4 doctype found... controlled by TY_(IsHTML5Mode)(doc)...

And as far as I understand it there are quite a number of table element attributes declared obsolete in html5, not just the valign shown, but step by step...

Would really appreciate some W3C references on this issue, rather than w3schools, but thanks for these...

To take it a step further, would be really nice if given the -clean option, tidy convert them to CSS, but that could be a separate issue...

Look forward to comments, patches, PR's to correct this situation... thanks...

@geoffmcl geoffmcl added this to the 5.7 milestone Apr 4, 2018
@petdance
Copy link
Contributor Author

petdance commented Apr 4, 2018

Would really appreciate some W3C references on this issue, rather than w3schools, but thanks for these...

Sorry about that. I didn't know where to find them. If you could find a corresponding link that would have been better and paste it here, I'll have a starting point for next time.

@ler762
Copy link
Contributor

ler762 commented Apr 4, 2018

@geoffmcl W3C reference
https://www.w3.org/TR/2014/NOTE-html5-diff-20141209/#obsolete-attributes
Some attributes from HTML4 are no longer allowed in HTML.

or up a level for new/changed/obsolete:
https://www.w3.org/TR/2014/NOTE-html5-diff-20141209/#language
This section is split up in several subsections to more clearly illustrate the various differences from HTML4.

@geoffmcl
Copy link
Contributor

geoffmcl commented Apr 5, 2018

@ler762 thank you for the beautiful HTML5 obsolete attributes references... and this got me to dig deeper into this... Wow, this literally opens up a can-of-worms ;=)) In more ways than one...

I started to build a HTML5 document, adding some of these obsolete attributes, just to begin to see what tidy does, and more importantly does not do, and struck a snag on even the first rev and charset attributes on link and a.

First it seems the rev is also not flagged by the nu validator... ok, maybe needs time to catch up...

And then started to find inconsistencies in the W3C documentation in that rev is still listed for a link and an a element, at least in the big single page HTML 5.2 14 December 2017 document... ok, they too can take time to catch up...

Thankfully it seems charset has been removed, and is flagged by nu, but not by tidy, unless - see below...

So with ongoing inconsistencies like this maybe we should concentrate on those where it is clear... but this should probably be a separate issue...

But back to this specific valign case...

I started to look at how tidy verifies an attribute. Part of the giant element table in tidy, Dict tag_defs[], is a list of attributes allowed on that element, and each those attributes each have a list of allowed versions, but by default, that check is not done, and maybe the attribute table, for each element is not entirely up-to-date... but...

But, for example, processing the above sample input, my in_711.html, with the option --strict-tags-attributes yes will yield a very different output -

F:\Projects\tidy-test\test\input5>tidy5 -v
HTML Tidy for Windows version 5.7.3
F:\Projects\tidy-test\test>tidy5 --strict-tags-attributes yes input5\in_711.html
line 8 column 13 - Error: <tr> attribute "valign" not allowed for HTML5
line 9 column 17 - Error: <th> attribute "valign" not allowed for HTML5
line 12 column 17 - Error: <td> attribute "valign" not allowed for HTML5
Info: Document content looks like HTML5
Tidy found 0 warnings and 3 errors!

This document has errors that must be fixed before
using HTML Tidy to generate a tidied up version.

That's right, tidy will flag these as errors if --strict-tags-attributes yes option is used. Sorry I did not think of this option earlier, but it is there, and has been there since circa Jan 2016 - since about version 5.1.33, so is in the 5.2, 5.4, and 5.6 releases... see #346 PR #350

In essence this particular issue seems closed - just use the --strict-tags-attributes yes option.

Tidy more than complains about valign, it flags it as an ERROR! Now there could be discussion about whether it should be an error or a warning, and whether this option should be changed to default to yes. but maybe that should be separate issue, discussion...

Note if you add a legacy doctype, like my in_711-1.html, and still use the strict option, tidy will pass it - no warnings or errors - as will the legacy W3C validator...

And this is not to say that there maybe other obsolete attributes that tidy may miss, but that must be a separate issue...

So @petdance can this be closed? Thanks...

@petdance
Copy link
Contributor Author

petdance commented Apr 5, 2018

Sure, if --strict-tags-attributes yes works, then I'm happy.

@petdance petdance closed this as completed Apr 5, 2018
@geoffmcl geoffmcl mentioned this issue May 14, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Technical Discussion
Projects
None yet
Milestone
5.7
Development

No branches or pull requests
3 participants
@petdance @geoffmcl @ler762