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.

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

--strict-tags-attributes no doesn't ignore <td align> #729

Closed
petdance opened this issue May 11, 2018 · 5 comments
Closed

--strict-tags-attributes no doesn't ignore <td align> #729

petdance opened this issue May 11, 2018 · 5 comments
Labels

Comments

@petdance
Copy link
Contributor

petdance commented May 11, 2018

This looks like it may be related to #688.

HTML file with an old table in it.

$ cat -n foo.html
     1  <!DOCTYPE html>
     2  <html>
     3      <head>
     4          <title> Alignment </title>
     5      </head>
     6      <body>
     7
     8          <table>
     9              <tr valign="bottom">
    10                  <td width="50" align="center">
    11                      text
    12                  </td>
    13              </tr>
    14          </table>
    15
    16      </body>
    17  </html>

tidy complains with --strict-tags-attributes yes about <tr valign>, <td width> and <td align>, as would expect.

$ tidy -q -e --strict-tags-attributes yes foo.html
line 9 column 13 - Error: <tr> attribute "valign" not allowed for HTML5
line 10 column 17 - Error: <td> attribute "width" not allowed for HTML5
line 10 column 17 - Error: <td> attribute "align" not allowed for HTML5
This document has errors that must be fixed before
using HTML Tidy to generate a tidied up version.

However, with --strict-tags-attributes no, or with no option, it still complains about <td align> even though it has NOT complained about the <td width>.

$ tidy -q -e --strict-tags-attributes no foo.html
line 10 column 17 - Warning: <td> attribute "align" not allowed for HTML5
$ tidy -q -e foo.html
line 10 column 17 - Warning: <td> attribute "align" not allowed for HTML5

I'm expecting that tidy will not complain about <td align> in the bottom two cases.

@petdance petdance added the Bug label May 11, 2018
@petdance petdance changed the title --strict-tags-attributes no doesnt' ignore <td align> --strict-tags-attributes no doesn't ignore <td align> May 11, 2018
@geoffmcl
Copy link
Contributor

@petdance thank you for the issue...

As partially discussed in #688, I see this as a deeper problem, but need feedback to decide...

Simply, in a HTML5 document, I think the output with --strict-tags-attributes yes is ok, but with the current default of no, I think the same 3 messages should still be output as warnings, just not as errors...

The reasoning is, if the user does not request strict tag-attribute compliance, then they should be just warnings. Most browsers will still honor them, regardless of doctype...

But in all cases they should be warned to fix it... this output is done in the phase 2, tidyDocCleanAndRepair section of the code...

The W3C nu validator will report, Error: The **valign/width/align** attribute on the **tr/td** element is obsolete. Use CSS instead.... i.e in all three cases...

Of course, change the doctype to say legacy 4.01, and both tidy and the W3C legacy validator will pass it with no problems... where the phase 2 CheckHTML5 service is not run...

I'm expecting that tidy will not complain about <td align> in the bottom two cases.

So, to be clear, I think this expectation is wrong. The user should be warned to not use these 3 attributes in a HTML5 document full stop!

The request for strict-tags-attributes should only change from a warning to an error, but the 3 messages should stay... unless specifically muted...

Now that is the coding I would look at, but not in a vacuum, i.e. no feedback, and what I would expect from a PR by others... thanks...

@petdance
Copy link
Contributor Author

The request for strict-tags-attributes should only change from a warning to an error

I guess that's OK with me, although I'm not sure I understand the difference between a warning an error if it's strictly diagnostic.

geoffmcl added a commit that referenced this issue May 14, 2018
@geoffmcl
Copy link
Contributor

@petdance yes, in your advised use case of only wanting the warnings/error reported, like with -e -q, then it is the same, unless you also monitor, and have use for, the exit value of tidy...

With only warnings it will be 1, with errors it will be 2, else 0, unless some major error, like can't read file, etc..., when it will be <0... and with 2 there will be no html output unless you add --force-output yes...

This option strict-tags-attributes came about in commit 2ade335, Feb 2016, maybe #350, #346, #372, #370, #711, and/or others, and reading back throught most of them now, while I tried to participate in some of the discussion back then, I guess I did not understand, or appreciate, all the changes made, and the consequences...

Have created an issue-729 branch with a fix as I see it... essentially, as indicated above...

Now running this new tidy in default mode, which is --strict-tags-attributes no, using my in_729.html, based on your sample, I will get, with -q -e -

line 10 column 1 - Warning: <td> attribute "align" not allowed for HTML5
line 9 column 1 - Warning: <tr> attribute "valign" not allowed for HTML5
line 10 column 1 - Warning: <td> attribute "width" not allowed for HTML5
line 10 column 1 - Warning: <td> attribute "align" not allowed for HTML5

Now this has the small problem that the align warning is repeated twice, so must try to fix that...

Then adding --strict-tags-attributes yes will get, as before -

line 9 column 1 - Error: <tr> attribute "valign" not allowed for HTML5
line 10 column 1 - Error: <td> attribute "width" not allowed for HTML5
line 10 column 1 - Error: <td> attribute "align" not allowed for HTML5
This document has errors that must be fixed before
using HTML Tidy to generate a tidied up version.

Then changing to doctype to 4.01 transitional, in_729-1.html, and 4.01 strict, in_729-2.html, get no warnings, errors, even with --strict-tags-attributes yes, and --doctype strict, as expected...

And no problems using this new tidy on the regression tests, although there are some tests that include these attributes - 433021, 433666, 441508, 588061 - and maybe others...

I tried one fix to get rid of the duplication, which is first output in CheckHTML5, but this then caused regressions on the above 4 tests, so must look harder for this...

Anyway, my issue-729 branch seems a step in the right direction... feedback, patches, PR welcome... thanks...

@geoffmcl
Copy link
Contributor

Have created PR #928 ... please test, report, comment... thanks...

balthisar added a commit that referenced this issue Jun 30, 2021
Is #729 - Show 'warnings' in all td cases
@balthisar
Copy link
Member

Closing due to merging #928.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants