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

Dimensions: Add offset prop fallback to FF for unreliable TR dimensions #4808

Merged
merged 2 commits into from
Jan 11, 2021

Conversation

timmywil
Copy link
Member

@timmywil timmywil commented Nov 10, 2020

Fixes gh-4529

Summary

Firefox incorrectly (or perhaps correctly) includes table borders in computed
dimensions, but they are the only one. Workaround this by testing for it and
falling back to offset properties.

This applies to latest FF and so needs to go on master as well.

Checklist

+203 bytes

Firefox incorrectly (or perhaps correctly) includes table borders in computed
dimensions, but they are the only one. Workaround this by testing for it and
falling back to offset properties

Fixes jquerygh-4529
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support tests are coming back! Logic looks good to me, I just hope we can limit the size increase a bit. Some of it is expected, modifying an existing test is cheaper than creating a new one when there's none at the moment.

I added some comments.

src/css.js Show resolved Hide resolved
src/css/support.js Show resolved Hide resolved
src/css/support.js Outdated Show resolved Hide resolved
src/css/support.js Outdated Show resolved Hide resolved
@mgol mgol added Dimensions Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Nov 10, 2020
@mgol mgol added this to the 4.0.0 milestone Nov 10, 2020
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one doubt about a comment. Thanks! 🙂

Comment on lines +149 to +154
// 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back when we had lots of support comment, when code was guarded by one, we usually didn't comment what gets worked around as the support comment above the support test was explaining that anyway. Maybe we can remove this comment in that case here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, I see we have a similar comment on 3.x-stable here so I'm fine either way.

@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Dec 7, 2020
@timmywil timmywil merged commit 3bbbc11 into jquery:master Jan 11, 2021
@timmywil timmywil deleted the 4529-ff-table-borders-master branch January 11, 2021 16:56
@parallels999
Copy link

@timmywil is #5270 related?

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

Successfully merging this pull request may close these issues.

Firefox 70 fails the "dimensions: width/height on a table row with phantom borders (gh-3698)" test
3 participants