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

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

Closed
mgol opened this issue Oct 22, 2019 · 9 comments · Fixed by #4808

Comments

@mgol
Copy link
Member

mgol commented Oct 22, 2019

Description

The just released Firefox 70 (& newer) fails the "dimensions: width/height on a table row with phantom borders (gh-3698)" test. As Travis uses the newest version of Firefox, it started failing on master.

Link to test case

See the relevant test case in the jQuery repo, a screenshot:
Screen Shot 2019-10-22 at 20 00 16

@timmywil
Copy link
Member

Just a note, we keep track of swarm failures in the meeting notes. Those tests can be in flux a lot between releases and I don't think we need to create a GH issue for each failure. All failures are always addressed before a release can go out.

@mgol
Copy link
Member Author

mgol commented Oct 22, 2019

This doesn't fail on Swarm as we don't have Firefox 70 there yet; it fails on Travis as I've recently added Firefox testing there & Firefox 70 has been released today.

I think these issues make sense; they e.g. allow to assign an issue to a particular person, they also make it easier to distinguish flakes from consistent issues (this is the latter case). It looks like a real issue to me, it fails on stable, beta & nightly and I meant to look at it tomorrow.

@mgol
Copy link
Member Author

mgol commented Oct 23, 2019

To clarify, I'm not saying we have to create issues for all those TestSwarm failures. But we can if we feel the need IMO.

@mgol
Copy link
Member Author

mgol commented Oct 23, 2019

One additional reason to allow issue creation in such cases is that this creates a space to dump the investigation results which I plan to do soon.

@mgol
Copy link
Member Author

mgol commented Oct 23, 2019

Results from my investigation:
The failing test case was added in #3738; it works on the following HTML:

<table id="gh3698" style="border-collapse: separate; border-spacing: 0; background: #baffc9"><tbody>
	<tr style="margin: 0; border: 10px solid black; padding: 0">
		<td style="margin: 0; border: 0; padding: 0; height: 42px; width: 42px;"></td>
	</tr>
</tbody></table>

and runs .outerWidth() & .outerWidth( true ) on the tr element. In all our tested browsers, including Chrome 78 & Firefox <70, it returns 42, with the exception of Firefox 70 where it returns 62. The issue is that Firefox 70 started reporting 10px for getComputedStyle( tr ).borderLeftWidth (the same for borderRightWidth); Firefox 68 reports 0px. However, Chrome already reports 10px as well... So why did it start breaking? Because in Firefox (both 68 & 70) getComputedStyle( tr ).width is 42px while in Chrome it's 22px - it used to add up to the same number even though partial results were different.

An interesting consequence of that is that in both Firefox 68 & 70 $( tr ).innerWidth() returns 42px while in Chrome it returns 22px - we just don't have a test that would report this difference.

Safari behaves just like Chrome while Edge & IE 11 returns auto for getComputedStyle( tr ).width - but we already work around that. getComputedStyle( tr ).borderLeftWidth is 10px in all browsers (Chrome 78, Firefox 70, Safari 13, Edge 18 & IE 11) except Firefox 68 so that change actually aligns them more with others.

@mgol
Copy link
Member Author

mgol commented Oct 23, 2019

Reported to Firefox at https://bugzilla.mozilla.org/show_bug.cgi?id=1590837.

mgol added a commit to mgol/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Ref jquery#4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
mgol added a commit to mgol/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Ref jquery#4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
mgol added a commit that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Closes gh-4537
Ref #4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
@mgol mgol removed the Needs review label Oct 28, 2019
@mgol
Copy link
Member Author

mgol commented Oct 28, 2019

Removing the "Needs review" label but leaving the bug open - we want to wait see what the resolution of w3c/csswg-drafts#4444 and then most likely write a support test & work around the behavior in non-conforming browsers.

For now, the respective test was just disabled in Firefox.

mgol added a commit that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Closes gh-4537
Ref #4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444

(cherry picked from commit c79e1d5)
@gibson042
Copy link
Member

gibson042 commented Nov 5, 2019

I have been dreading this because table layout is just so special, but the reality is that we're surprisingly consistent with all browsers except for Firefox.

https://output.jsbin.com/siqolaxipa

browser border-collapse CSS widths offsetWidth jQuery width/inner/outer
IE 11 separate 2×┃10px┃ auto 42 22 ≤ 22 ≤ 42
Edge 18 separate 2×┃10px┃ auto 42 22 ≤ 22 ≤ 42
Chrome 70 separate 2×┃10px┃ 22px 42 22 ≤ 22 ≤ 42
Safari 11.1 separate 2×┃10px┃ 22px 42 22 ≤ 22 ≤ 42
Safari 12.1 separate 2×┃10px┃ 22px 42 22 ≤ 22 ≤ 42
Android 9 separate 2×┃10px┃ 22px 42 22 ≤ 22 ≤ 42
iOS 12 separate 2×┃10px┃ 22px 42 22 ≤ 22 ≤ 42
Firefox 69 separate 2×┃ 0px┃ 42px 42 42 ≤ 42 ≤ 42
Firefox 70 separate 2×┃10px┃ 42px 42 42 ≤ 42 ≤ 62
-- -- -- -- --
IE 11 collapse 2×┃10px┃ auto 32 12 ≤ 12 ≤ 32
Edge 18 collapse 2×┃10px┃ auto 32 12 ≤ 12 ≤ 32
Chrome 70 collapse 2×┃10px┃ 12px 32 12 ≤ 12 ≤ 32
Safari 11.1 collapse 2×┃10px┃ 12px 32 12 ≤ 12 ≤ 32
Safari 12.1 collapse 2×┃10px┃ 12px 32 12 ≤ 12 ≤ 32
Android 9 collapse 2×┃10px┃ 12px 32 12 ≤ 12 ≤ 32
iOS 12 collapse 2×┃10px┃ 12px 32 12 ≤ 12 ≤ 32
Firefox 69 collapse 2×┃ 0px┃ 32px 32 32 ≤ 32 ≤ 32
Firefox 70 collapse 2×┃10px┃ 32px 32 32 ≤ 32 ≤ 52

Collapsed-borders mode doesn't even come close to matching the CSS box model we assume in jQuery dimensions, so I think that can be thrown out for this discussion.

But in separated-borders mode, we have the right .outerWidth() for all browsers except Firefox 70 and the wrong .width() and .innerWidth() for all browsers except Firefox.

To fix .outerWidth(), I think we should be pragmatic and update the reliableTrDimensions test so that it additionally reports false for Firefox 70 (e.g., when the rounded sum of computed height and computed border width does not equal offsetHeight), sacrificing fractional pixels for computed dimensions of table rows (but not for their containers or children, where dimensions are more relevant).

I don't have any good ideas to fix .width() and .innerWidth() right now, but maybe it's fine for them to be broken in both table borders modes.

@timmywil
Copy link
Member

timmywil commented Dec 2, 2019

We'll at least do a PR for outerWidth.

@mgol mgol removed this from the 3.5.0 milestone Mar 16, 2020
@mgol mgol added this to the 3.6.0 milestone Mar 16, 2020
@timmywil timmywil assigned timmywil and unassigned mgol Nov 2, 2020
timmywil added a commit to timmywil/jquery that referenced this issue Nov 9, 2020
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
timmywil added a commit to timmywil/jquery that referenced this issue Nov 10, 2020
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
timmywil added a commit that referenced this issue Jan 11, 2021
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 gh-4529
Closes gh-4808
timmywil added a commit that referenced this issue Jan 11, 2021
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 gh-4529
Closes gh-4807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment