web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
4.98k stars 3.09k forks source link

Incorrect test added for text-transform #46490

Open frivoal opened 4 months ago

frivoal commented 4 months ago

cc: @fred-wang, @mrego , @xiaochengh (as authors / reviewers)

As far as I can tell, https://github.com/web-platform-tests/wpt/blob/master/css/css-text/text-transform/text-transform-upperlower-107.html is in direct contradiction with the spec text.

https://drafts.csswg.org/css-text/#text-transform-property says:

This property transforms text for styling purposes. It has no effect on the underlying content, and must not affect the content of a plain text copy & paste operation.

But the test checks the opposite: that the copied text is affected by the transform.

Please be careful when writing / reviewing tests to look at the specification and confirm that it matches the behavior you expect, otherwise this can be very misleading / confusing for other people later on.

In this case, should we simply revert the test (and the corresponding implementation change), or do you want to open a bug against the spec to argue that it should be changed?

cc: @fantasai (as co-editor)

fred-wang commented 4 months ago

@frivoal Thanks. I believe this was discussed elsewhere (probably https://github.com/w3c/csswg-drafts/issues/3775), but chromium's implementation purposely disagrees with the spec and AFAIK there is no plan to change the implementation. So indeed this shouldn't have landed into the official repo but we need to keep this and probably similar tests for Chromium. So probably the plan forward is to move these tests into wpt_internal (and similar internal repositories for other browsers).

fred-wang commented 4 months ago

cc @bkardell

bkardell commented 4 months ago

Note: It passes in its current form in WebKit as well, so it seems that by far the vast, vast majority of users currently experience the opposite of what the spec says? I'm curious if there are bugs from users suggesting that it's wrong?

frivoal commented 4 months ago

Firefox does what the spec say, and as far as I know it doesn't have bugs reported against it.

In any case, I know where I stand in this debate, but if disagreement persists, we should argue about it and about what the spec should say, not introduce test that encourage others to ignore the spec and do something else.

fred-wang commented 4 months ago

not introduce test that encourage others to ignore the spec and do something else.

I assume you didn't mean to suggest that this was our intention when we introduced these tests, but just repeating myself: this is a mistake and such a test should really be kept in the internal ones (and it is important for chromium to ensure copying text-transformed text is not broken).

adampage commented 4 months ago

Coincidentally, I added a similar WPT test a couple days ago to investigate the effect of text-transform in the computing of an element’s accessible name. I also filed an accompanying issue in w3c/accname, where I think a clarification will be beneficial either way.

The bleeding of text-transform into the browser’s accessibility tree feels to me like a straightforward violation of its specified purpose and more likely to harm a user’s experience than help it. But I also now understand that accessibility folks have previously debated this and some have advocated for text-transform to convey semantics. In any case, all 3 major engines do currently expose the transformed text.

All the while, I was unaware of this similar WPT issue and all the discourse around the expectations for this feature. 😅

I suppose I don’t have anything new to add here, but will be watching this issue as I consider whether I need to remove my own WPT test or invert their expectations.

frivoal commented 4 months ago

I assume you didn't mean to suggest that this was our intention […]

I did not, and I apologize for the phrasing.

yisibl commented 4 months ago

Please note: Chrome changed the behavior of copying to the clipboard a while ago, and now text-transform does not affect the actual text copied to the clipboard.

fred-wang commented 3 months ago

@adampage @yisibl Thanks for the context information, it seems we should keep an eye on this. It seems the copy-and-paste thing is still experimented under a flag and the a11y part is to be discussed again.

There are a few text-transform tests using selection/copy. As I see ./text-transform-upperlower-107.html and text-transform-math-auto-003.html assume the transformed text is copied. Will change the expectation to copying the original text and keep their current versions as internal tests. text-transform-upperlower-105.html and text-transform-upperlower-106.html just checks that selecting the DOM text is properly reflected in the background of the rendered text so they look fine (the failure on 106 seems a bug in safari).

https://wpt.fyi/results/css/css-text/text-transform?label=master&label=experimental&aligned&q=text-transform-upperlower-105.html%7Ctext-transform-upperlower-106.html%7Ctext-transform-upperlower-107.html%7Ctext-transform-math-auto-003.html

$ css/css-text/text-transform$ find -type f | xargs grep 'getSelection()'
./math/text-transform-math-auto-003.html:            .getSelection()
./math/text-transform-math-auto-003.html:            window.getSelection().toString(),
./text-transform-upperlower-107.html:    window.getSelection().setBaseAndExtent(target, 0, target, 1);
./text-transform-upperlower-107.html:    assert_equals(window.getSelection().toString(), "SS");
./text-transform-upperlower-106.html:  window.getSelection().setBaseAndExtent(target.firstChild, 0, target.firstChild, 3);
./text-transform-upperlower-105.html:  window.getSelection().setBaseAndExtent(target, 0, target, 1);
./reference/text-transform-upperlower-106-ref.html:  window.getSelection().setBaseAndExtent(target.firstChild, 0, target.firstChild, 2);
./reference/text-transform-upperlower-105-ref.html:  window.getSelection().setBaseAndExtent(target, 0, target, 1);
$ css/css-text/text-transform$ find -type f | xargs grep 'toString()'
./math/text-transform-math-auto-003.html:    <meta name="assert" content="Verify Selection.toString() on a character with 'text-transform: math-auto' returns the transformed unicode character.">
./math/text-transform-math-auto-003.html:            window.getSelection().toString(),
./math/text-transform-math-auto-003.html:        }, `Selection.toString() for math-auto '${String.fromCodePoint(code_point)}' returns the transformed character.`);
./text-transform-upperlower-107.html:    assert_equals(window.getSelection().toString(), "SS");
./text-transform-upperlower-107.html:  }, "Selection.toString() for 'ß' with text-transform: uppercase");
fred-wang commented 3 months ago

Please note: Chrome changed the behavior of copying to the clipboard a while ago, and now text-transform does not affect the actual text copied to the clipboard.

@yisibl I noticed you commented there are more cases where the transformed text is used instead of the original text: https://issues.chromium.org/issues/40343523#comment32 ; Selection.toString() which is used by the tests I mentioned, don't seem to be affected by this behavior change.

fred-wang commented 3 months ago

As far as I can tell, https://github.com/web-platform-tests/wpt/blob/master/css/css-text/text-transform/text-transform-upperlower-107.html is in direct contradiction with the spec text.

https://drafts.csswg.org/css-text/#text-transform-property says:

This property transforms text for styling purposes. It has no effect on the underlying content, and must not affect the content of a plain text copy & paste operation.

But the test checks the opposite: that the copied text is affected by the transform.

So checking again ./text-transform-upperlower-107.html and text-transform-math-auto-003.html rely on Selection.toString() which is actually not performing a copy and paste operation, and so not covered by this quotation from the css text spec and not affected by Chromium's new behavior (flag: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/runtime_enabled_features.json5;l=2148)

Instead, it returns a string with the result of https://w3c.github.io/selection-api/#dom-selection-stringifier :

The stringification must return the string, which is the concatenation of the rendered text if there is a range associated with this.

I'm not sure what is referred to by "rendered text" but it looks like it should have the transform applied and so Chromium's behavior aligns with the spec.