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

[css-flexbox] issue with the references for gap-006-*? #26326

Closed
foolip opened this issue Oct 29, 2020 · 10 comments · Fixed by #26371
Closed

[css-flexbox] issue with the references for gap-006-*? #26326

foolip opened this issue Oct 29, 2020 · 10 comments · Fixed by #26371

Comments

@foolip
Copy link
Member

foolip commented Oct 29, 2020

These are the tests in question: gap-006-lr.html, gap-006-ltr.html, gap-006-rl.html, gap-006-rtl.html

I'm interested in the Firefox failure, but included WebKitGTK and Servo results in case they provide clues. Here are the (cropped) actual and expected for gap-006-ltr.html in Firefox:

image

image

I suspect the problem here is really with the reference here, because when I open https://wpt.live/css/css-flexbox/gap-006-ltr.html or https://wpt.live/css/css-flexbox/gap-006-ltr-ref.html in Chrome or Safari, they look like the actual Firefox rendering.

The reference also uses Flexbox, so there's probably an interop issue here, but actually not with gap?

@argyleink (test author) @davidsgrogan @aethanyc would any of you like to take a look?

@argyleink
Copy link
Contributor

afaict, this is an interop issue that's unrelated to gap and is closer to flex alignment

@foolip
Copy link
Member Author

foolip commented Oct 29, 2020

Can the references here be turned into test (for flex alignment) in themselves and the references changes to use some other technique that will be consistent?

@davidsgrogan
Copy link
Member

So what we need to do is

(1) Take the current gap-006-ltr reference, make it a separate wpt and figure out why engines don't render it the same.
(2) Change the gap-006-* tests to not be affected by this difference.

@argyleink do you want to do this or do you want me to?

@argyleink
Copy link
Contributor

image

Firefox Nightly not showing the issue for me, and it should be right? I can't make any of my browser versions reproduce the issue shown in the WPT tests reference. Is anyone able to recreate the issue in a local environment?

@davidsgrogan
Copy link
Member

Yeah, I see the problem in Firefox nightly. Maybe that means it's a text measurement thing that is environment dependent.
Screen Shot 2020-10-29 at 2 15 37 PM

@argyleink
Copy link
Contributor

confirmed, i can reproduce only with Firefox Nightly on Linux

@argyleink
Copy link
Contributor

nightly-flexbox

@foolip
Copy link
Member Author

foolip commented Oct 29, 2020

@stephenmcgruer is there a straightforward way to tell if this test has always been failing in Firefox, to rule out that it's just a regression in Nightly? (If it is then maybe the test could be left unchanged.)

@davidsgrogan
Copy link
Member

I recall that this has always been failing, but I am interested to hear how to query for that info :)

@davidsgrogan
Copy link
Member

And, I'm 99% sure this isn't a bug in Firefox, just an unfortunate effect of Chrome and Firefox giving the default font different sizes.

We'll fix the reference to work in both browsers.

chromium-wpt-export-bot pushed a commit that referenced this issue Nov 3, 2020
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in #26326

Fixes web-platform-tests/wpt/#26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 3, 2020
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in #26326

Fixes #26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 4, 2020
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in #26326

Fixes #26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 4, 2020
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in #26326

Fixes #26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Nov 4, 2020
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in web-platform-tests/wpt#26326

Fixes web-platform-tests/wpt#26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 6, 2020
…in firefox, a=testonly

Automatic update from web-platform-tests
[css-flex] Fix some gap tests that fail in firefox

These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in web-platform-tests/wpt#26326

Fixes web-platform-tests/wpt#26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}

--

wpt-commits: b9dea15062d348abb8da77996e8eb0bd4f9e5dec
wpt-pr: 26371
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Nov 10, 2020
…in firefox, a=testonly

Automatic update from web-platform-tests
[css-flex] Fix some gap tests that fail in firefox

These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in web-platform-tests/wpt#26326

Fixes web-platform-tests/wpt#26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}

--

wpt-commits: b9dea15062d348abb8da77996e8eb0bd4f9e5dec
wpt-pr: 26371
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
These failed due to firefox and chrome using different default fonts. I
avoided that problem by assigning a fixed width to the divs containing
the words.

These tests now pass when I run them locally in firefox with
./wpt run --binary ~/ff-nightly/firefox firefox css/css-flexbox/gap-006*

This was filed in web-platform-tests/wpt#26326

Fixes web-platform-tests/wpt#26326

Change-Id: Ib0795c67bce5f80f7a33e1da74a026c1ed574b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515226
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823830}
GitOrigin-RevId: da0f2c9fda27a7cc85f122857c694353cb7ff77a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@foolip @argyleink @davidsgrogan and others