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

Firefox: outerHeight() not taking <caption> height into account for tables #4105

Closed
Payn opened this Issue Jun 16, 2018 · 12 comments

Comments

Projects
None yet
5 participants
@Payn
Copy link

Payn commented Jun 16, 2018

Description

outerHeight() returns only <table> height even if it contains <caption> in Firefox 61, IE 11 and Edge.
Works fine in Chrome

Link to test case

https://jsfiddle.net/a6c4jdt3/6/

outerHeight() returns 100 instead of 130

@1jj

This comment has been minimized.

Copy link

1jj commented Jul 11, 2018

This should be fixed in Firefox 63
https://bugzilla.mozilla.org/show_bug.cgi?id=820891

@timmywil timmywil added this to the Future milestone Sep 3, 2018

@TruptiM18

This comment has been minimized.

Copy link

TruptiM18 commented Jan 13, 2019

Can I work on this issue?

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jan 13, 2019

@TruptiM18 Sure, feel free! Have a look at our Contributing guide if you need a place to start https://github.com/jquery/jquery/blob/master/CONTRIBUTING.md.

@timmywil timmywil closed this Jan 13, 2019

@timmywil timmywil reopened this Jan 13, 2019

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jan 13, 2019

The bugzilla link above says the bug is fixed in Firefox, but I don't see any difference in FF 64 on Windows. Could anyone else confirm this is still an issue?

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jan 13, 2019

Nvm, updated test case. It seems native offsetHeight does take caption into account, but jQuery's outerHeight does not (note that jQuery does in Chrome). It's because we're using computed height, which still does not include caption in Firefox only. https://jsfiddle.net/timmywil/Lqrwhnxu/7/

@timmywil

This comment has been minimized.

Copy link
Member

timmywil commented Jan 15, 2019

Opened new bug on Firefox to see if we can get this fixed in native computedStyle. https://bugzilla.mozilla.org/show_bug.cgi?id=1520264

@TruptiM18

This comment has been minimized.

Copy link

TruptiM18 commented Jan 19, 2019

@timmywil I tested the test case code in following browsers on Windows and results are mentioned below-
Microsoft Edge 42
jQuery outerHeight 100
window.getComputedStyle(elem).height 100
getBoundingClientRect 130
offsetHeight 130
clientHeight 130
scrollHeight 130

Chrome 71
jQuery outerHeight 130
window.getComputedStyle(elem).height 128
getBoundingClientRect 130
offsetHeight 130
clientHeight 130
scrollHeight 130

IE 11
jQuery outerHeight 130
window.getComputedStyle(elem).height 98
getBoundingClientRect 130
offsetHeight 130
clientHeight 130
scrollHeight 130

Firefox 61
jQuery outerHeight 100
window.getComputedStyle(elem).height 100
getBoundingClientRect 130
offsetHeight 100
clientHeight 100
scrollHeight 100

The above mentioned issue with jQuery's outerHeight() does not exist with IE 11 on Windows when I tested it.
As you mentioned, Firefox does not take caption height into account while returning offsetHeight.

Also, there are discrepancies in the values returned by getComputedStyle().height in different browsers-
Chrome:
getComputedStyle().height does consider caption height and returns 130.

IE:
getComputedStyle().height of IE 11 does not consider caption height and also subtracts the border height (-2px) from the rectangles height 100 and returns 98 instead of 100.

Firefox:
getComputedStyle().height of Firefox considers only the height of rectangle and returns 100.

Can we open a bug to IE team as getComputedStyle(elem).height returned by IE is different than the other browsers?

Please let me know if Mozilla team is not updating their code and we need a code change for this issue.

Thanks.

@mgol mgol added the Needs review label Jan 20, 2019

@mgol

This comment has been minimized.

Copy link
Member

mgol commented Jan 20, 2019

I added the "Needs review" label to discuss latest arguments from the discussion in the Firefox ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=1520264

@mgol

This comment has been minimized.

Copy link
Member

mgol commented Jan 20, 2019

@TruptiM18 Thanks for testing it in various browsers!

Can we open a bug to IE team as getComputedStyle(elem).height returned by IE is different than the other browsers?

It doesn't buy us anything to report an issue to IE as IE is never going to change again, it's a frozen legacy browser and we'll have to live with its bugs until it dies.

Edge is soon going to switch to Chromium so it only really makes sense to submit issues to Chrome, Firefox or Safari (or rather WebKit) bug trackers.

@TruptiM18

This comment has been minimized.

Copy link

TruptiM18 commented Jan 21, 2019

@mgol Thanks for the explanation!

@mgol

This comment has been minimized.

Copy link
Member

mgol commented Feb 11, 2019

We've discussed it today. While it may look surprising that an element may be higher than its set height, its not really isolated behavior. With the default content-box value for box-sizing specifying a padding or border on an element will cause that behavior already. It turns out tables have similar behavior - captions make the table higher than the specified height.

Now, outerHeight is supposed to cover padding & border (and optionally even margin) but it doesn't say anything about <caption>. Also, what if someone really wants the current behavior? Should we introduce a separate flag that will handle caption? This is all introducing complexity for an edge case as captions are way less prevalent than paddings or borders.

Also, as Boris Zbarsky rightly noticed in https://bugzilla.mozilla.org/show_bug.cgi?id=1520264#c1/show_bug.cgi?id=1520264, Chrome behavior has an unfortunate behavior where setting elem.style.height to the value returned from the same element with getCompitedStyle(elem).height would change its height.

Therefore, it seems we might be closer to accept Firefox behavior. But since the spec seems unclear what's desired here and browsers disagree, we wouldn't want to end up in a situation where we implement behavior opposite to the one that's specced. Until that's resolved, we're not going to normalize either way.

@mgol mgol closed this Feb 11, 2019

@mgol mgol removed this from the Future milestone Feb 11, 2019

@mgol mgol added the wontfix label Feb 11, 2019

@mgol

This comment has been minimized.

Copy link
Member

mgol commented Feb 11, 2019

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