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

[forms] vertical writing-mode tests seem suspicious. #294

Closed
emilio opened this issue Mar 1, 2023 · 8 comments · Fixed by web-platform-tests/wpt#39004
Closed

[forms] vertical writing-mode tests seem suspicious. #294

emilio opened this issue Mar 1, 2023 · 8 comments · Fixed by web-platform-tests/wpt#39004
Labels
focus area: Forms test-change-proposal Proposal to add or remove tests for an interop area

Comments

@emilio
Copy link

emilio commented Mar 1, 2023

Test List

These tests:

Rationale

Seem backwards? I'd expect a vertical range (like Firefox does), but Firefox is the only engine failing the test?

cc @nt1m who created these tests.

@emilio emilio added the test-change-proposal Proposal to add or remove tests for an interop area label Mar 1, 2023
@jfkthame
Copy link

jfkthame commented Mar 1, 2023

It looks like the reason Firefox is failing these is that direction: rtl is not having any effect on the vertical range control, which it probably should.

On the other hand, Safari and Chrome both render a horizontal control, which seems even more wrong, but the test doesn't detect this as a failure.... :-(

@nt1m
Copy link
Member

nt1m commented Mar 1, 2023

It looks like the reason Firefox is failing these is that direction: rtl is not having any effect on the vertical range control, which it probably should.

This is precisely what this is testing.

Seem backwards? I'd expect a vertical range (like Firefox does), but Firefox is the only engine failing the test?

I think Chrome was failing this test too with their vertical form controls enabled (maybe it's no longer the case?). WebKit passes with the vertical form controls enabled.

On the other hand, Safari and Chrome both render a horizontal control, which seems even more wrong, but the test doesn't detect this as a failure.... :-(

Yeah I agree that should be detected as a failure, happy to take suggestions on how to fix the test. Potentially a new mismatch? or something that turns red? not sure what's the most simple path here.

@jfkthame
Copy link

jfkthame commented Mar 1, 2023

Presumably we could at least assert a mismatch against a testcase that doesn't have vertical writing-mode, but is otherwise the same.

@nt1m
Copy link
Member

nt1m commented Mar 1, 2023

I'll take a look next week, but I'm happy to review PRs if one comes first.

Also side note about vertical form controls, most of the tests were me and @dizhang168's work so we'd appreciate if Gecko has any new tests to contribute to the tests from their own implementation experience of vertical form controls :)

@jfkthame
Copy link

jfkthame commented Mar 1, 2023

So this raises the question in my mind: which direction should an inline-oriented range control progress by default in vertical writing mode? (Without RTL getting involved.)

In Gecko, for a vertical range control in horizontal writing mode, it progresses from bottom to top (like the mercury going up a thermometer).

Currently, in vertical writing mode (e.g. Japanese with writing-mode: vertical-rl) we use the same rendering, which means the range will progress "upstream" against the inline flow of the text. I think that seemed reasonable, given that (AFAIK) the Japanese don't normally hold their thermometers with the bulb at the top and the mercury expanding downwards.

But if that's so, then what about RTL directionality in vertical writing modes, e.g. an Arabic or Hebrew section within a vertical Japanese page? If we now reverse the direction of the range, it'll be progressing downwards, against the inline direction of the RTL text; and that doesn't feel very natural to me.

So my gut feeling is that in vertical writing mode, the vertical range should always progress upwards, regardless of the inline direction. But that's not what these tests are asserting.

WDYT? (Does any spec discuss this?) Should we actually reverse the "default" for vertical writing modes, so that an inline-oriented range control within vertical Japanese would progress downwards? What does webkit do? (I don't seem to have support for this in the version of Safari Tech Preview that I have here.)

@nt1m
Copy link
Member

nt1m commented Mar 1, 2023

There's whatwg/html#8413 opened to discuss this.

I personally think we should just follow the inline flow direction (and this is what WebKit does, you can enable "Vertical form controls" from experimental feature flags to test), since there doesn't seem to be any agreement on what's the best experience for each language.

@dizhang168
Copy link
Member

The reason it is passing in chrome right now is because it is defaulting to the horizontal behavior. To make these tests fail consistently when not correctly implemented, maybe whenever we are testing range-input-appearance-*-vertical-*.optional.html, we add the line <link rel="mismatch" href="range-input-appearance-*-horizontal*.optional.html">. And vice versa.

On a side note, some of these writing mode tests made it to Interop 2023 and I wonder if we should change them to be not optional anymore.

@nt1m
Copy link
Member

nt1m commented Mar 15, 2023

I submitted a PR to fix this: web-platform-tests/wpt#39004

nt1m added a commit to web-platform-tests/wpt that referenced this issue Mar 15, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 30, 2023
…tl.optional.html tests more strict, a=testonly

Automatic update from web-platform-tests
Make range-input-appearance-*-vertical-rtl.optional.html tests more strict (#39004)

Fixes web-platform-tests/interop#294
--

wpt-commits: b44dce82e3a3d0790d76e90360ea0c91fca94482
wpt-pr: 39004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: Forms test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants