Skip to content

Reconsider the width/height css hooks CSS swap behavior #5628

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

Closed
mgol opened this issue Feb 11, 2025 · 1 comment · Fixed by #5630 or #5634
Closed

Reconsider the width/height css hooks CSS swap behavior #5628

mgol opened this issue Feb 11, 2025 · 1 comment · Fixed by #5630 or #5634
Assignees
Milestone

Comments

@mgol
Copy link
Member

mgol commented Feb 11, 2025

Description

Summary

We have width & height CSS hooks. The get one:

jquery/src/css.js

Lines 316 to 335 in 667321e

get: function( elem, computed, extra ) {
if ( computed ) {
// Certain elements can have dimension info if we invisibly show them
// but it must have a current display style that would benefit
return rdisplayswap.test( jQuery.css( elem, "display" ) ) &&
// Support: Safari <=8 - 12+, Chrome <=73+
// Table columns in WebKit/Blink have non-zero offsetWidth & zero
// getBoundingClientRect().width unless display is changed.
// Support: IE <=11+
// Running getBoundingClientRect on a disconnected node
// in IE throws an error.
( !elem.getClientRects().length || !elem.getBoundingClientRect().width ) ?
swap( elem, cssShow, function() {
return getWidthOrHeight( elem, dimension, extra );
} ) :
getWidthOrHeight( elem, dimension, extra );
}
},

checks for display to be none or to start with table- with some exceptions and then it applies some temporary styles:
cssShow = { position: "absolute", visibility: "hidden", display: "block" },

before measuring the size.

The display: none case

Now, removing this swap doesn't make any test fail in Firefox or Chrome and only one, related to table columns, in Safari. We don't test the display: none case. I've been wondering if that one makes any sense. Even just applying position: absolute may affect the element styles beyond what we'd see if we just gave it a temporary display: block. There's no way to always properly measure hidden elements. Can we get rid of this one in 4.0?

Table columns

As for table columns, the support comment states:

// Support: Safari <=8 - 12+, Chrome <=73+
// Table columns in WebKit/Blink have non-zero offsetWidth & zero
// getBoundingClientRect().width unless display is changed.
// Support: IE <=11+
// Running getBoundingClientRect on a disconnected node
// in IE throws an error.

Modern versions of Safari or Chrome do not match what the comment says. I noticed different cross-browser issues, though. For a column with span="2" (denoted as c) that is sized implicitly by the size of its cells, I got the following values in an example table:

Chrome Safari Firefox IE 11
c.offsetWidth 172 172 172 0
c.getBoundingClientRect().width 171.59375 0 171.61666870117188 0
getComputedStyle(c).width "171.594px" "0px" "83.65px" "auto"

Some conclusions:

  • Chrome reports all the values and they are roughly equal - barring any transforms that would affect getBoundingClientRect.
  • Safari only has a non-zero offsetWidth - the comment stated that one was 0 as well.
  • Firefox reports all the values but getComputedStyle(c).width only reports the size of the first column over which the col element spans.
  • In IE, none of these values makes sense. If the col element has an explicit width set via CSS then IE starts reporting it instead of "auto". However, our cssShow styles have no effect on reported values.

Proposal

It seems in Chrome we could report getComputedStyle(c).width as we'd normally do, but Safari & Firefox need offsetWidth for different reasons; perhaps we need a new support test. The getBoundingClientRect examples were provided just for comparison but we cannot use them as they're affected by transforms.

Either way, swapping styles doesn't seem necessary. I'm not sure if we can do anything for IE here either, but we're not doing it at the moment.

Incidentally, this is our last usage of the swap util, previously exposed as jQuery.swap.

Link to test case

https://output.jsbin.com/pebovey

@mgol mgol added CSS Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Feb 11, 2025
@mgol mgol self-assigned this Feb 11, 2025
mgol added a commit to mgol/jquery that referenced this issue Feb 13, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 13, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
@mgol
Copy link
Member Author

mgol commented Feb 13, 2025

PR: #5630

mgol added a commit to mgol/jquery that referenced this issue Feb 13, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 18, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
@mgol mgol added this to the 4.0.0 milestone Feb 19, 2025
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE, `<col>` computed width is `"auto"` unless `width` is set explicitly via
CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Don't try to show elements hidden via `display: none` before measuring their
   width or height. The styles we used to apply included `position: absolute`
   which could influence the styles anyway, so this was never reliable.
2. Fix measurements of `<col span="2">` elements in Firefox.
3. Avoid swapping CSS before dimensions computation in all browsers.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Don't try to show elements hidden via `display: none` before measuring their
width or height. The styles we used to apply included `position: absolute`
which could influence the styles anyway, so this was never reliable.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Feb 24, 2025
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Fix measurements of `<col span="2">` elements in Firefox.
2. Fix measurements of all implicitly sized `<col>` elements in Safari.

Firefox always reports computed width as if `span` was 1. In Safari, computed
width for columns is always 0. Work around both issues by using `offsetWidth`.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Fix measurements of `<col span="2">` elements in Firefox.
2. Fix measurements of all implicitly sized `<col>` elements in Safari.

Firefox always reports computed width as if `span` was 1. In Safari, computed
width for columns is always 0. Work around both issues by using `offsetWidth`.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628
mgol added a commit to mgol/jquery that referenced this issue Feb 24, 2025
Changes:
1. Fix measurements of `<col span="2">` elements in Firefox.
2. Fix measurements of all implicitly sized `<col>` elements in Safari.

Firefox always reports computed width as if `span` was 1. In Safari, computed
width for columns is always 0. Work around both issues by using `offsetWidth`.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes jquerygh-5628

CSS: Support dimensions for implicitly sized hidden elements again
@mgol mgol modified the milestones: 4.0.0, 3.7.2 Feb 24, 2025
@mgol mgol linked a pull request Feb 24, 2025 that will close this issue
1 task
@mgol mgol closed this as completed in eca2a56 Feb 24, 2025
mgol added a commit that referenced this issue Feb 24, 2025
Changes:
1. Fix measurements of `<col span="2">` elements in Firefox.
2. Fix measurements of all implicitly sized `<col>` elements in Safari.

Firefox always reports computed width as if `span` was 1. In Safari, computed
width for columns is always 0. Work around both issues by using `offsetWidth`.

In IE/Edge, `<col>` computed width is `"auto"` unless `width` is set explicitly
via CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.

Fixes gh-5628
Closes gh-5634
Ref gh-5630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment