CSS: Fix dimensions of table <col> elements
#5630
Conversation
5db4c9a to
0c60c0c
Compare
|
I recommend reviewing with whitespace changes disabled: https://github.com/jquery/jquery/pull/5630/files?w=1 |
|
A test run on BrowserStack for this PR was green: https://github.com/mgol/jquery/actions/runs/13318861255/job/37199296893 |
0c60c0c to
eb9b57d
Compare
| // Support: IE 10 - 11+ | ||
| // IE misreports `getComputedStyle` of table rows with width/height | ||
| // set in CSS while `offset*` properties report correct values. | ||
| // Support: Firefox 70+ | ||
| // Firefox includes border widths | ||
| // in computed dimensions for table rows. (gh-4529) |
There was a problem hiding this comment.
We already have these comments next to the support test so we don't need to repeat them here.
| swap( elem, cssShow, function() { | ||
| return getWidthOrHeight( elem, dimension, extra ); | ||
| } ) : | ||
| getWidthOrHeight( elem, dimension, extra ); |
There was a problem hiding this comment.
We could potentially inline this function here now but I wanted to avoid a big diff.
gibson042
left a comment
There was a problem hiding this comment.
I'm in favor of fixing measurement of column-spanning elements in Firefox and table-related elements in general, but opposed to removing the style swap for display: none. Measuring such elements by swapping in position: absolute is not guaranteed to be accurate, but it is useful, particularly when preparing for visibility (e.g., at the start of an animation). This is one of those clunky-to-implement but valuable-to-have features that I think is worth preserving.
|
@gibson042 I'll admit this was added before my time so I may be lacking context. Is this logic useful mostly in cases where element dimensions are explicitly set via CSS (inline or selector-based)? Otherwise, it seems setting |
|
@gibson042 I'm also asking because if EDIT: This also explains why removing this fallback didn't make our tests fail. We already have a test for a hidden div with an explicitly set |
7c8dc21 to
ebb1689
Compare
|
@timmywil @gibson042 this is ready for another review. I'm considering whether to backport this to |
79d909d to
04b6443
Compare
display: none ones<col> elements
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
04b6443 to
714d51e
Compare
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
Summary
Changes:
<col span="2">elements in Firefox.<col>elements in Safari.Don't try to show elements hidden viabacked outdisplay: nonebefore measuring their width or height. The styles we used to apply includedposition: absolutewhich could influence the styles anyway, so this was never reliable.Firefox always reports computed width as if
spanwas 1. In Safari, computedwidth for columns is always 0. Work around both issues by using
offsetWidth.In IE/Edge,
<col>computed width is"auto"unlesswidthis set explicitlyvia CSS so measurements there remain incorrect. Because of the lack of a proper
workaround, we accept this limitation.
Fixes gh-5628
Size diff:
Checklist
If needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com