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

Table row height reported incorrectly in Edge #4490

Closed
TPIvan opened this issue Sep 26, 2019 · 8 comments · Fixed by #4506
Closed

Table row height reported incorrectly in Edge #4490

TPIvan opened this issue Sep 26, 2019 · 8 comments · Fixed by #4506

Comments

@TPIvan
Copy link

@TPIvan TPIvan commented Sep 26, 2019

Description

When css height is specified on tr element (<tr id="myrow" style="height:1px;">) .height() returns this height instead of actual height. The problem is present in Microsoft Edge (42.17134, 44.18362) but not present in Google Chrome ( 77.0.3865.90 ) and Mozilla Firefox (69.0.1). The problem is present since jquery 3.3.0 and was reproduced on earlier versions. And is present on current git version (4.0.0-pre 50871a5)

How to reproduce. Create a table with one row (tr) and one cell (td) with text in the cell. Set the row height to 1px (style="height:1px;"). JQuery .height() on the row should return at least font height but actually returns 1 in Edge.

Link to test case

https://codepen.io/TPIvan/pen/aboxZaQ

@mgol
Copy link
Member

@mgol mgol commented Sep 26, 2019

Thanks for the issue & the test case! I recreated it on JS Bin as CodePen doesn't support IE: https://output.jsbin.com/xusurij/1.

The issue exists on IE 9-11 10-11 (IE 9 looks fine) as well.

@mgol
Copy link
Member

@mgol mgol commented Sep 26, 2019

The issue is that getComputedStyle on a tr with height set via inline styles returns that inline style, disregarding the internal contents. getBoundingClientRect doesn't have that issue, neither does offsetHeight. In IE/Edge we seem to have a choice between occasionally incorrect height on table rows and losing fractional values on those rows.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Sep 30, 2019

getBoundingClientRect is the wrong tool for the job of CSS box model introspection, and we were right to abandon it. But offset{Width,Height} seem like acceptable graceful degradations to use.

@mgol mgol self-assigned this Sep 30, 2019
@mgol mgol added this to the 3.5.0 milestone Sep 30, 2019
@hichemsmairia
Copy link

@hichemsmairia hichemsmairia commented Oct 3, 2019

whats the problem here guys

@timmywil
Copy link
Member

@timmywil timmywil commented Oct 4, 2019

@hichemsmairia @mgol describes the issue above. IE10, 11, and Edge report the wrong value using getComputedStyle in this use case. We are considering using offset{Width,Height} instead, but it does have the downside of rounding to the nearest whole number value.

@zzz6519003

This comment has been minimized.

@mgol
Copy link
Member

@mgol mgol commented Oct 7, 2019

PR for 3.x-stable: #4503

mgol added a commit to mgol/jquery that referenced this issue Oct 7, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 8, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 8, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
mgol added a commit to mgol/jquery that referenced this issue Oct 9, 2019
@mgol
Copy link
Member

@mgol mgol commented Oct 14, 2019

Fix landed on master in 26415e0 & on 3.x-stable in 6d31477.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

6 participants