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

Breaking change on height(Upgrading from 3.5.1 to 3.6/3.7) #5270

Closed
parallels999 opened this issue Jun 15, 2023 · 7 comments · Fixed by #5278 or #5279
Closed

Breaking change on height(Upgrading from 3.5.1 to 3.6/3.7) #5270

parallels999 opened this issue Jun 15, 2023 · 7 comments · Fixed by #5278 or #5279

Comments

@parallels999
Copy link

parallels999 commented Jun 15, 2023

Using .height() is rounding the value

  • What do you expect to happen?
    get the real height
  • What actually happens?
    the browser shows the real height but jquery get it rounded
  • Which browsers are affected? tested on
    chrome 114.0.5735.110
    opera 99.0.4788.65
    chromium:113.0.5672.127
    edge 114.0.1823.43
    firefox 103.0.2

Description

having rounded height produces misaligned objects

V3.5:
image
V3.6/V3.7:
image

Link to test case

Same code, just different versions of jquery
V3.7.0: https://jsfiddle.net/6jqh4pmb/1/ (fails)
V3.6.4: https://jsfiddle.net/6jqh4pmb/2/ (fails)
V3.5.1: https://jsfiddle.net/6jqh4pmb/3/ (This is working fine)

@parallels999
Copy link
Author

parallels999 commented Jun 16, 2023

UPDATE:

Apparently this issue is shown when using bootstrap(3/4/5), bootstrap add this css:

* { box-sizing: border-box; }

jQuery works again if i add this css

tr { box-sizing: content-box; }

but since this worked correctly in 3.5.1 for me it is a bug,
It also fails with other css plugins, but I haven't figured out which styling affects it.

Working Demo: https://jsfiddle.net/hc3ym1Lt/
Failing Demo: https://jsfiddle.net/hc3ym1Lt/1/
Without Bootstrap Demo: https://codepen.io/parallels999/pen/WNYwxVG
image

@timmywil timmywil added Bug Dimensions Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Jun 16, 2023
@timmywil
Copy link
Member

Thanks for opening an issue! It turns out our support test is not accounting for global border-box box-sizing. We can fix this by fixing the support test.

@timmywil timmywil added Blocker and removed Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Jun 26, 2023
@timmywil timmywil added this to the 3.7.1 milestone Jun 26, 2023
@mgol
Copy link
Member

mgol commented Jun 26, 2023

Note: we will fix it only in Chrome & Safari. In Firefox & IE it will remain an integer. This is because those browsers fail the reliableTrDimensions support test and we need to use outerHeight as a workaround - which does not return fractional values.

If Firefox ever starts passing the support test, it will automatically get fractional values here.

@mgol mgol self-assigned this Jun 26, 2023
mgol added a commit to mgol/jquery that referenced this issue Jun 26, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

This threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
mgol added a commit to mgol/jquery that referenced this issue Jun 26, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

This threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
mgol added a commit to mgol/jquery that referenced this issue Jun 26, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

This threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5278
@mgol
Copy link
Member

mgol commented Jun 26, 2023

mgol added a commit to mgol/jquery that referenced this issue Jun 26, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5278
mgol added a commit to mgol/jquery that referenced this issue Jun 26, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5279
mgol added a commit to mgol/jquery that referenced this issue Jun 29, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5279
mgol added a commit to mgol/jquery that referenced this issue Jun 29, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS is the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5278
mgol added a commit to mgol/jquery that referenced this issue Jul 7, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS are the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5278
mgol added a commit to mgol/jquery that referenced this issue Jul 7, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS are the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5279
mgol added a commit that referenced this issue Jul 10, 2023
…SS (3.x version)

Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS are the same as on a page without it.

Fixes gh-5270
Closes gh-5279
Ref gh-5278
mgol added a commit to mgol/jquery that referenced this issue Jul 10, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS are the same as on a page without it.

Fixes jquerygh-5270
Ref jquerygh-5279
mgol added a commit that referenced this issue Jul 10, 2023
Bootstrap 5 includes the following CSS on the page:

```css
*,
*::before,
*::after {
  box-sizing: border-box;
}
```

That threw our `reliableTrDimensions` support test off. This change fixes the
support test and adds a unit test ensuring support test values on a page
including Bootstrap 5 CSS are the same as on a page without it.

Fixes gh-5270
Closes gh-5278
Ref gh-5279
@cyfung1031
Copy link

cyfung1031 commented Mar 25, 2024

This is still not making sense in 3.7.1

https://jsfiddle.net/lunarlogic489/Las4vtbz/

Screen Shot 2024-03-25 at 19 38 33

Why tr height can be larger than tbody height? (Firefox)

@timmywil
Copy link
Member

The difference was noted in a previous comment #5270 (comment). Further explanation in your issue.

@mgol
Copy link
Member

mgol commented Mar 29, 2024

It's a bit unfortunate that we penalize Firefox here though its behavior is technically correct according to the current spec. That said, there's a spec issue at w3c/csswg-drafts#4444 that is still open; the discussion there suggests the current spec may not be what's wanted and may need to change. Changing this behavior would be a breaking change in jQuery and there's a risk we'd have to change back once the spec issue is clarified. That's why I'd prefer to wait with changes here until after the spec issue is clarified. It seems the discussion there stalled, though.

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