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

Fix behavior of <button> elements in HTML 5 #531

Closed
wants to merge 1 commit into from

Conversation

lhchavez
Copy link
Contributor

HTML 5 restricts the children of elements to only phrasing
content. HTML 4 also allowed block elements.

In particular, this change allows code such as

<span><button>OK</button></span>

to be left as-is, since the does not need duplication for
crossing a block boundary. Mitigates #461.

HTML 5 restricts the children of <button> elements to only phrasing
content. HTML 4 also allowed block elements.

In particular, this change allows code such as

    <span><button>OK</button></span>

to be left as-is, since the <span> does not need duplication for
crossing a block boundary. Mitigates htacg#461.
@lhchavez
Copy link
Contributor Author

PTAL! This was tested against geoffmcl/tidy-test@4f6089b and passes.

@geoffmcl
Copy link
Contributor

As replied to #530...

@geoffmcl geoffmcl added this to the 5.5 milestone Apr 25, 2017
@balthisar
Copy link
Member

Confirmed as working on macOS.

@geoffmcl
Copy link
Contributor

geoffmcl commented May 2, 2017

@lhchavez sorry for the delay on this...

I was working on an alternate patch, to put this change of the parser for <buttom> in TY_(ResetTags) - reset to html5, and TY_(AdjustTags) - set to html4 mode, if a legacy doctype is found. As you will see this is where several other such mode swithes are handled...

And have pushed my patch to the issue-461 branch for testing...

This would keep all the switches between html5 and htm4 mode in one place, rather than creating a new parser just for <button>, which does the same thing - switches between ParseInline and ParseBlock...

But ran into regression testing problems, in that my fix caused quite a number of diffs, and some exit value changes... Arrgh!!!

On investigating this, it seems some errors have crept into the regression tests!

Now most of the regression tests are still legacy documents. Take say 1086083. It is HTML 4.01, and testcase contains the code <li>Huh?</li>, and tidy is supposed to fix this by adding a <ul>, which, if we back up the tidy-tests repo is what we will find in the original out_1086083.html, but not in the current expects/case-1086083.html!

In other words, this expects/case-1086083.html now fails validation with the error Line 16, Column 6: document type does not allow element "LI" here; missing one of "UL", "OL" start-tag! While the original out_1086083.html passes!!

That is we now expect, allow some html5 tolerance in a legacy document, and this is an error! And there seems to be several other similar cases... yet to be explored fully...

Now this has nothing to do with <button>, or #461, and may in fact merge this, as is, until I can find time to investigate and correct the current expects, in a separate issue... please bear with me... thanks...

@geoffmcl
Copy link
Contributor

geoffmcl commented May 2, 2017

PS: Now opened Issue 15...

Any help appreciated... thanks...

@geoffmcl
Copy link
Contributor

geoffmcl commented May 6, 2017

@lhchavez as agreed have merged #540 and closed #461, so closing this...

@geoffmcl geoffmcl closed this May 6, 2017
@lhchavez lhchavez deleted the fix-button-html5 branch May 6, 2017 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants