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

Don't auto-insert tbody? #1835

Closed
mgol opened this Issue Nov 4, 2014 · 12 comments

Comments

Projects
None yet
6 participants
@mgol
Member

mgol commented Nov 4, 2014

Now that we're talking breaking changes - jQuery inserts a tbody when appending to an empty table because of a "special" behavior of IE6-7 (http://bugs.jquery.com/ticket/14290). The master branch does the same for compat with jQuery 1.x. Should we revert this behavior now that we're dropping IE<8?

@mgol mgol added this to the 3.0.0 milestone Nov 4, 2014

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 4, 2014

Member

@dmethvin This would be a good ticket for the needs review label.

Also, if the behavior isn't needed in any of our supported browsers, let's drop it.

Member

timmywil commented Nov 4, 2014

@dmethvin This would be a good ticket for the needs review label.

Also, if the behavior isn't needed in any of our supported browsers, let's drop it.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 4, 2014

Member

I recall @markelog trying to remove more of that in the early days of jQuery 2.0, resulting in http://bugs.jquery.com/ticket/13232 . If more can be removed safely I'm all for it.

Member

dmethvin commented Nov 4, 2014

I recall @markelog trying to remove more of that in the early days of jQuery 2.0, resulting in http://bugs.jquery.com/ticket/13232 . If more can be removed safely I'm all for it.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Nov 4, 2014

Member

I think it should be done (if it can be done) after officially dropping IE6-7 so that we can cleanly cherry-pick the change between branches.

Member

mgol commented Nov 4, 2014

I think it should be done (if it can be done) after officially dropping IE6-7 so that we can cleanly cherry-pick the change between branches.

mgol added a commit to mgol/jquery that referenced this issue Nov 4, 2014

WIP: Misc: Drop support for older browsers; update support comments
That includes IE<8, Opera 12.x, Firefox<29, Safari<6.0 and some hacks
for old Blackberry.

Fixes gh-1835
Fixes gh-1701
Refs gh-1815

mgol added a commit to mgol/jquery that referenced this issue Nov 4, 2014

Misc: Drop support for older browsers; update support comments
That includes IE<8, Opera 12.x, Firefox<29, Safari<6.0 and some hacks
for old Blackberry.

Fixes gh-1835
Fixes gh-1701
Refs gh-1815

mgol added a commit to mgol/jquery that referenced this issue Nov 4, 2014

Misc: Drop support for older browsers; update support comments
That includes IE<8, Opera 12.x, Firefox<29, Safari<6.0 and some hacks
for old Blackberry.

Fixes gh-1835
Fixes gh-1701
Refs gh-1815
Refs gh-1820
@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 4, 2014

Member

Good call.

Member

timmywil commented Nov 4, 2014

Good call.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jan 7, 2015

Member

@markelog Do you recall the issues you encountered with the tbody problem? Maybe I'm remembering it wrong.

Member

dmethvin commented Jan 7, 2015

@markelog Do you recall the issues you encountered with the tbody problem? Maybe I'm remembering it wrong.

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Jan 7, 2015

Member

That was a long time ago, a bit busy today, will refresh my memory tomorrow

Member

markelog commented Jan 7, 2015

That was a long time ago, a bit busy today, will refresh my memory tomorrow

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Jan 15, 2015

Member

Now that we're talking breaking changes

Doesn't look like one

because of a "special" behavior of IE6-7

Not exactly

http://bugs.jquery.com/ticket/14290

Don't understand how this related, it just references tbody creation

I recall @markelog trying to remove more of that in the early days of jQuery 2.0

Was not involve in it, you probably thinking of 19def21.

<a href="According to html4 spec, tbody should always be present, which is true for IE6-7 implementation, tbody will be auto-inserted if table created through html serialization (declared in html or through innerHTML property), regardless if content is present or not. tbody should be created if DOM methods are used, otherwise IE6-7 will improperly render it.

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

Modern browsers and IE8+ do not create tbody through html serialization for empty table and behave according to specification if there is a specific content, with DOM-methods, tbody will not be created, regardless of the content, even Opera 9 and FF 3.6 behave this way.

When tbody creation is nessary? For IE6-9, when empty table is statically created and innerHTML property is used, you basically can't use innerHTML on that node in these environments.

We no longer use innerHTML directly on tables (even true for jQuery#html method), so this code-path could be safely removed.

Member

markelog commented Jan 15, 2015

Now that we're talking breaking changes

Doesn't look like one

because of a "special" behavior of IE6-7

Not exactly

http://bugs.jquery.com/ticket/14290

Don't understand how this related, it just references tbody creation

I recall @markelog trying to remove more of that in the early days of jQuery 2.0

Was not involve in it, you probably thinking of 19def21.

<a href="According to html4 spec, tbody should always be present, which is true for IE6-7 implementation, tbody will be auto-inserted if table created through html serialization (declared in html or through innerHTML property), regardless if content is present or not. tbody should be created if DOM methods are used, otherwise IE6-7 will improperly render it.

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

Modern browsers and IE8+ do not create tbody through html serialization for empty table and behave according to specification if there is a specific content, with DOM-methods, tbody will not be created, regardless of the content, even Opera 9 and FF 3.6 behave this way.

When tbody creation is nessary? For IE6-9, when empty table is statically created and innerHTML property is used, you basically can't use innerHTML on that node in these environments.

We no longer use innerHTML directly on tables (even true for jQuery#html method), so this code-path could be safely removed.

@markelog markelog self-assigned this Jan 19, 2015

markelog added a commit to markelog/jquery that referenced this issue Jan 21, 2015

markelog added a commit to markelog/jquery that referenced this issue Jan 21, 2015

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Jan 21, 2015

Member

When tbody creation is nessary? For IE6-9, when empty table is statically created and innerHTML property is used, you basically can't use innerHTML on that node in these environments.

A bit more info about this is here

Member

markelog commented Jan 21, 2015

When tbody creation is nessary? For IE6-9, when empty table is statically created and innerHTML property is used, you basically can't use innerHTML on that node in these environments.

A bit more info about this is here

@fdlk

This comment has been minimized.

Show comment
Hide comment
@fdlk

fdlk Jan 23, 2016

For the record, this is a breaking change, affecting the layout of our table in modern browsers.

fdlk commented Jan 23, 2016

For the record, this is a breaking change, affecting the layout of our table in modern browsers.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jan 23, 2016

Member

@fdlk can you open a new issue that includes a test case?

Member

dmethvin commented Jan 23, 2016

@fdlk can you open a new issue that includes a test case?

@fdlk

This comment has been minimized.

Show comment
Hide comment
@fdlk

fdlk Jan 25, 2016

@dmethvin I've created #2861 with a jsfiddle to demonstrate.

fdlk commented Jan 25, 2016

@dmethvin I've created #2861 with a jsfiddle to demonstrate.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Mar 6, 2016

Member

This has been reverted so I'm removing the milestone.

Member

mgol commented Mar 6, 2016

This has been reverted so I'm removing the milestone.

@mgol mgol removed this from the 3.0.0 milestone Mar 6, 2016

@cssmagic cssmagic referenced this issue May 18, 2016

Open

jQuery #5

@gibson042 gibson042 added this to the 3.0.0 milestone Feb 12, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.