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

Sanitizer and Chrome output totally different html when tr is inside td #135

Open
topcoderKai opened this issue Jan 12, 2018 · 4 comments
Open

Comments

@topcoderKai
Copy link

Sanitizer outputs html different than Chrome when tr is inside td. And the output affects the layout of html pages dramatically.
Test case:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>
<body>
<div id="DESKTOP">
    <table width="100%" border="0" cellspacing="0" cellpadding="0" bgcolor="#ffffff" align="center">
      <tr>
        <td align="center" valign="top" width="100%">
       <tr>
   </table>
</div>
<div id="MOBILE" style="display:none; width:0px; max-height:0px; overflow:hidden;">mobile contents</div>
</body>
</html>

Chrome output:

<html><head></head><body>
<div id="DESKTOP">
    <table width="100%" border="0" cellspacing="0" cellpadding="0" bgcolor="#ffffff" align="center">
      <tbody>
        <tr>
          <td align="center" valign="top" width="100%">
         </td>
       </tr>
       <tr>
       </tr>
     </tbody>
    </table>
</div>
<div id="MOBILE" style="display:none; width:0px; max-height:0px; overflow:hidden;">mobile contents</div>
</body></html>
@topcoderKai
Copy link
Author

topcoderKai commented Jan 12, 2018

The Sanitizer output:

<div id="DESKTOP">    
  <table width="100%" border="0" cellspacing="0" cellpadding="0" bgcolor="#ffffff" align="center">
    <tbody>
      <tr>
        <td align="center" valign="top" width="100%">       
          <table>
            <tbody>
              <tr></tr>
            </tbody>
          </table>
          <div id="MOBILE" style="display:none; width:0px; max-height:0px; overflow:hidden;">mobile contents</div>
        </td>
      </tr>
    </tbody>
  </table>
</div>

@topcoderKai
Copy link
Author

topcoderKai commented Jan 12, 2018

Looks like the issue is caused by impliedElements handling. Not sure what's the reason of handling the cases this way and result differently than Chrome? Why not just keep the contents unchanged and left the tag matching to the browser to handle?

@topcoderKai
Copy link
Author

Looks like the table insertion does not follow the html spec here https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intd. Any reason that the sanitizer does not follow the spec and has it's own way of dealing with incomplete/mis-nested table elements @mikesamuel?

@mikesamuel
Copy link
Contributor

Duplicate of #137

@mikesamuel mikesamuel marked this as a duplicate of #137 Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants