-
Notifications
You must be signed in to change notification settings - Fork 222
The layout of a html page which has nested <a> inside tables was messed up #123
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
I'm not going to download and unzip attachments from people I don't know onto my local machine. Please put testcases in the bug text using an HTML code section like ```html Your HTML goes here ``` |
<a href="http://foo.example.com/">
<table><tr><td><a href="http://bar.example.com/">foobar</a></td></tr></table>
</a> The sanitizer moves the inner "a" element outside the table, which breaks the layout. This causes Netflix emails to display badly if they are run through the sanitizer. |
Thanks. Getting the semantics of nested I suppose I could try and do less damaging recovery. IIUC, the output you probably see is <a href="foo.example.com">
<table></tr><td></td></tr></table></a><a href="http://bar.example.com/">foobar</a> Would it be sufficient if I just closed the link on entering a table. <a href="http://foo.example.com/"></a>
<table><tr><td><a href="http://bar.example.com/">foobar</a></td></tr></table> That would mean the tags are still properly nested, but it would make it impossible for tables that are one giant link, which I, personally, am OK with. |
I think what's missing is the notion of a "marker" in the list of active formatting elements. https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intr At the start of a table cell, the parser should add a "marker" to the list of active formatting elements. When the parser then encounters an "a" element inside the table cell, there is no parse error because the parser is only looking for an existing "a" element between the end of the list and the last marker on the list. https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inbody |
I'm currently doing scope based reasoning in the element stack. I had assumed that the kind of reasoning that could be done with markers could be done with scopes. Is that wrong? |
I don't think it's wrong, but it requires extra care when checking if nested "a" elements are allowed. https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inbody says:
If there's a marker ahead of an "a" element in the list of active formatting elements, then we should allow that nesting of "a" elements. |
The test case:
|
any update on this? Ebay email html's also affected. |
We need a new test case. We don’t unzip untrusted files, please see below.
…--
Jim Manico
@manicode
Secure Coding Education
+1 (808) 652-3805
On Jan 8, 2018, at 9:35 AM, fr3akX ***@***.***> wrote:
any update on this? Ebay email html's also affected.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I posted the test case above, I hope I could simplify the test case more. Anyone have more simplified and better test case is appreciated. |
Duplicate of #137 |
There are nested
<a>
and<table>
in the test case and the sanitizer generated additional elementes such as</table>
and</a>
and messed up the layout.See the test case
tokenMessedup.html.zip
The text was updated successfully, but these errors were encountered: