-
Notifications
You must be signed in to change notification settings - Fork 429
Still seeing the warning "<table> lacks "summary" attribute" with HTML5 #377
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
Comments
@a3nm thanks for the report, with sample html - always appreciated... Yes, in #210 we suppressed that warning for HTML5, but tidy is viewing this document as XHTML5... replace the Looks like a simple fix... will get to it soonest, unless you, or others beat me to it with a patch, or PR... thanks.. |
Thanks for your answer! I confirm that with just Thanks if you can fix this! :) |
@a3nm as expected, tidy was only checking for Have applied a small patch to Could have also tested the version using But in reviewing other code I note several other places where we are only testing for the HTML5 version, but have left these as a new issues, as and when they come up. At that time maybe need a new service like Appreciated if you could pull, build and test this, and close this issue if satisfactory... thanks... |
I tried to pull and re-build, and I'm getting exactly the same error as before with the example document in my initial bug report. Is it working for you? |
@a3nm yes, it is working for me! Are you sure you pulled and built the Using an input: input5\in_377.html
Running tidy with a configuration
You can see the version I am using, 5.1.44. Note, adding a config of Please carefully check the version of tidy,
Can only suggest you are using an earlier version still! |
I confirm that I am running version 5.1.44:
With the document in your message, I do not get any error, but note that it is not the same as my original document as you added a My understanding is that the warning should not be emitted, even without the |
@a3nm thanks for the quick reply. Ok, we are now fine on the version ;=)) Hmmm, I added that meta tag because the W3C validator gave a warning about no charset... and it also pushed tidy to say -
Now without that meta tag tidy will take the document as XHTML 1.0 strict, and will thus emit the table summary warning...
The only change I made was to not emit it for HTML5 or XHTML5... So now the question is what is the W3C say on the summary attribute on a table for XHTML 1.0 strict? Is it a spurious warning? Need to research that... appreciate any links you find... thanks... |
From the XHTML 1.0 Strict DTD https://www.w3.org/TR/xhtml1/dtds.html#a_dtd_XHTML-1.0-Strict which appears to be what defines conformity to XHTML https://www.w3.org/TR/xhtml1/#docconf I observe that the "summary" attriibute of the "table" element is not mandatory (it is IMPLIED, not REQUIRED), so I don't think that a warning should be emitted for XHTML 1.0 Strict either. As for the question of the |
@a3nm thanks for looking that up... and yes, I understand the "summary" attribute on the "table" element is not mandatory in XHTML 1.0 strict... but it is allowed, and that does mean tidy will emit the warning... This warning was added by the original founders of tidy. They indicated it was there as an aid to the visually impaired, to descibe the table, like an accessibility warning. Over the early years I too argued against it, but did not get anywhere. And now that I am in that position of choice, am not yet convinced to suppress it. Concerning the meta charset tag, it was introduced in this form only in html5, previously as a meta content attribute. During the parsing tidy thus sees that new form constraining what the document type may be to HTML5 type, hence chooses HTML5 or XHTML5, and acts accordingly. You will note tidy does not warn about its omission, but perhaps should, like the W3C validator does. Following your links to the-meta-element I can read -
But that is a different question, as a separate issue. So concerning the summary warning, you obviously vote for suppression. What do other think? Remove it? In what document types? Maybe add yet another option like I will try to listen to reason, but as stated am not yet convinced to suppress it for any document less than HTML5 and now XHTML5. |
If the presence of an HTML5-specific element is good reason for tidy to consider the document as HTML5 (and emit HTML5-specific warnings), I wouldn't say that the absence of such elements should be legitimate cause for not considering as HTML5. I don't think tidy should emit warnings that would not be reasonable in HTML5 if the document can be seen as a valid HTML5 document. So I don't think the warning should be emitted on my original document, which reads fine as valid HTML5. As for the omission of the meta-element, as the Content-Encoding HTTP header is a legitimate mechanism, I think it is totally legitimate to omit it and tidy is right not to warn about it. That said, as a general comment, it would be nice to have a mechanism to toggle individual warnings from the command-line. If there were a clean way to ignore warnings that I dislike or don't care about, I would care less about tidy's default behavior. |
@a3nm yes, while I agree the absense of a HTML5-specific element is not a good reason for not considering the document as HTML5, the present code decision logic allows tidy to choose But please feel free to carefully examine this code logic, and present a patch, or a PR, showing a better way, without causing regressions when tidy is parsing obviously And at this time also agree, the omission of a content encoding header should not be a tidy warning, but just wish the W3C validator would agree also ;=)) but this is a separate subject! And it is also agreed, as a general comment, that having a myriad of command line options to toggle warnings would perhaps be a good thing!? I am immediately reminded of the gcc options, where for just about every warning, there is a BTW I will shortly be pushing a new option to tidy, Unfortuately, someone has to care about tidy's behaviour ;=)) |
I wonder if suppressing faults via a myriad of configuration options would be the best approach. This adds additional complexity to Tidy, of course, but it also seems that Tidy already provides a facility to filter unwanted messages: TidyReportFilter. There's probably a way to take advantage of this in the console application without having to build this into the library per se. (Actually it's not in the latest API documentation, but currently in master there's a better TidyReportFilter3.) Even in console this opens up a bigger question. Do we allow any error to be suppressed? Is this dangerous? Do we have a new command line switch for every possible error? Do we force it into another config file? Add it to the current config file? Right now there are 231 possible warnings/errors/info that can be output. |
@balthisar thank you for your comment... and separating it into two clear issues...
On the first libtidy should continue to report ALL warnings it sees during the parsing of the document. This should not change! It is correct that libtidy, if it Yes, others apps using libtidy have the choice to add various filters for this output... their choice... On the other hand, our sort of sample app, console tidy, should not have such filters. It should report what libtidy offers. Adding some 231 additional options to it would seem very wrong... even as a separate Now libtidy, from 5.1.44 onward, has been fixed to not report a warning if it finds the document as a So to me the original request has been met, and I am re-setting this as a Feature Request (FR), with an indefinite future (IF), until further comments convince me it is otherwise... Be aware, such FR-IF will be closed in the absence of further comments after a reasonable time... So please speak up... As always, thanks for your comments... |
I'm in alignment with @geoffmcl's comments. If no further discussion on the matter then we'll close this issue in the near future. |
The discussion has drifted, but I think the original bug that I was reporting has not yet been satisfactorily fixed: the document in my original report is, as far as I can tell, perfectly valid HTML5, and it still triggers a spurious warning when fed to tidy, even with the latest fix. I understand that the reason is that the document is mistakenly interpreted as XHTML 1.0 where it might make sense to advise in favor of that attribute. Nevertheless I think it is a bug that tidy emits a warning on a document if the document is valid when interpreted as HTML5. (As for the more general question of disabling warnings via command-line flags, as I user I think it would be useful, and not unreasonable, following gcc's example. Using flags from the shell is more convenient and modular than having to write code. But I agree that this is a more general question, certainly a feature request rather than a bug, and not related to my original issue.) |
Hmm... maybe this is a valid point if we use With Pinging @geoffmcl: what's your opinion on this? Respect the chosen doctype or not? There's likely to be other output affected... In any case I probably won't be able to have a close look at it until this weekend. And @a3nm, you're welcome to submit a patch! |
I agree that at least the behavior should be fixed for For automatic detection, I'm not sure I follow the explanation of @geoffmcl. Again, I find it weird that tidy would choose a doctype, think that it found a warning, and blame the user, whereas it should have given the user the benefit of doubt: tidy should not complain about an input document if there is some doctype for which the document is valid, in my opinion. I won't have time to submit a patch myself, I just want to point out the existence of the bug. |
@balthisar yes, I had thought of @a3nm it is not so much So as certain elements are found in the parsing, some bits are eliminated. The service is called Not yet checked exactly in code, but assume for example your But without that meta tag, or some other HTML5 only element, I guess XHTML 1.0 strict is also still in the bits remaining, and tidy chooses this, over XHTML5, which I think would also be there. While this choice could be altered I can hear others users suggesting I prefer strict, if it is available... It is not a question of But there is light at the end of the tunnel ;=))
By that I mean, what I shall call short form But one or both these could push tidy to choose only XHTML5. I will find time to look more closely at this, but as always patches, or a PR would be appreciated... thanks... |
Thanks a lot for your clarifications.
I think this would be a problem. As discussed earlier, this form of the HTML tag is perfectly valid in an HTML 5 document.
OK, I understand. Intuitively I would have expected that tidy by default would choose whatever doctype (if any) causes it to report no warning or errors. I would say that people who want to validate against a specific doctype to get more warnings could specify it with That said, this problem of which doctype to prefer is fuzzier. In any case, the warning in question should not be triggered with Thanks again for clarifying. |
@a3nm as mentioned that comment about kicking off non-xhtml bits was done without actually doing any debug, as is wrong, but the general idea of contraining remains valid... I added some more debug output, and first observing the dimishing bits, with the meta charset tag - input5\in_377.html
As you can see there was no change effected by the html element contrary to what I suggested. The fact that the document has a body, immediately removed all frameset bits after exit ParseHead. Then something else remove HT20, and no more until after exit ParseHTML, the end of the document parsing... Now we had four more constraints, the last, presumed the meta tag, kicked of everything except Now the sequences when there is not a meta charset tag - input5\in_377-3.html
Naturally, largely the same, except for what I presume was the last meta kicker, so this time tidy has 3 bits to choose from, and here, as we know, it chose Now as pointed out in commit 7bdc31a to at least not emit the warning for Perhaps I even need more debug to show exactly the reasons for each constraint... what element, attribute is the cause... Naturally I am only showing events where to call to the service actually caused some change... Anyway, will continue digging, especially how to incude at least As always ideas, discussion, patches, or a PR would be appreciated... thanks... |
@a3nm, have found a patch which will return XHTML5 before XHTML1, in specific circumstances, but need to do some more testing -
Meantime, since we are considering a release 5.2 shortly, changing the milestone here to 5.3... But if you can get a chance to add this patch and confirm it works, and if I get time for more testing then maybe it could be included in 5.2! |
Thanks! I just tested it out, and it doesn't work, sorry (the warning However I tested that adding "return HT50;" just before the patch Antoine Amarilli |
@a3nm you did add That is one of the required circumstances... thanks for testing... PS: Please try to remember to delete this text if you directly email reply... it just clutters up the issues if left... |
Indeed, with "--doctype html5" the warning is suppressed with the patch, I still stand by my opinion in the previous comments that the warning Thanks again! Antoine Amarilli |
@a3nm your persistence is winning through ;=)) If we assume Accordingly, have expand the patch, and fixed another little niggle - MSVC10 prefers explicit
Then, tidy in Appreciate you, and others, testing more and commenting... this could make it into 5.2 if all positive feedback... thanks... OT: Also love that if you add |
Hi, I can confirm that this patch makes the warning disappear both with and Antoine Amarilli |
@a3nm I like this fix because it again demonstrates the default html5 nature of tidy, while still handling legacy documents... Accordingly, have moved this back to a 5.2 milestone, and included it, bumping the version to 5.1.50... Perhaps this can now be closed? thanks... |
The latest master works for me, so as far as I'm concerned the issue can Antoine Amarilli |
The current master is still OK for me w.r.t. this issue. Thanks again! Antoine Amarilli |
Use the |
This is probably related to #210.
With the latest version from trunk and the following document:
I get the following warning when running
/usr/local/bin/tidy -utf8 -errors -quiet
:This HTML5 document is accepted by validator.w3.org without issues so I don't think this warning should be emitted.
The text was updated successfully, but these errors were encountered: