Skip to content

Conversation

@markelog
Copy link
Member

No description provided.

@markelog markelog force-pushed the 1835 branch 2 times, most recently from 8fb9650 to 3a7d0af Compare January 21, 2015 20:19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can either say "when it is present" or just "when present" for the descriptions on these.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old code, we guaranteed that a <tr> never got appended to a <table>. Do all the supported browsers in master auto-insert the <tbody> for us now? Do we have a unit test verifying that? I see some weak existing tests here and here but don't think they are making sure of this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you missed my comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I forgot you put in that much detail!

According to html5 spec, tbody could be ommited but will be auto-inserted if table has td, th, tr tags.

If so should we ensure that with a test? Is it true for Android 2.x for example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will complete my previous comment there.

Basically, you can't use innerHTML property on the table element (only on tbody) in IE6-9 (i suppose at one point innerHTML might have been used like that), plus, in IE6-7 it would protect you from the case like this -

$( "<table/>" ).append( "<tr>...</tr>" );

No need for that now.

Is it true for Android 2.x for example?

There could be situation when table exist with rows and columns without tbody parent - when you use DOM-manipulation methods. But it doesn't matter, since table should be rendered correctly with or without tbody in modern browsers.

Search for tbody though, still necessary, since if user created table through html serialization (like through innerHTML property or defining it directly in html) will create tbody element even if it was omitted, so in order to correctly and consistently manipulate that table, we still need to search for it.

And yeah, i checked it in Android 2.3... and IE6-7, Opera 9, Firefox 3.6 (just to understand genesis of that code-path). They all follow that logic.

@dmethvin
Copy link
Member

How could I ever doubt @markelog, this LTGM. 😺

@markelog markelog closed this in e984d1c Feb 11, 2015
markelog added a commit that referenced this pull request Feb 11, 2015
markelog added a commit that referenced this pull request Nov 10, 2015
@markelog markelog mentioned this pull request Dec 22, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants