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-text] Add lang=ja in css3-text-line-break-jazh tests #9805

Merged
merged 1 commit into from Mar 5, 2018

Conversation

Projects
None yet
6 participants
@mrego
Copy link
Member

mrego commented Mar 5, 2018

Without the lang attribute, the reference was choosing
a different character for English vs Japanese in some configurations.

The patch just adds "lang='ja'" to both the test and reference elements.

[css-text] Add lang=ja in css3-text-line-break-jazh tests
Without the lang attribute, the reference was choosing
a different character for English vs Japanese in some configurations.

The patch just adds "lang='ja'" to both the test and reference elements.

@wpt-pr-bot wpt-pr-bot added the css-text label Mar 5, 2018

@mrego mrego removed request for nox, fantasai, plinss, hakatashi and r12a Mar 5, 2018

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 5, 2018

Build PASSED

Started: 2018-03-05 11:44:47
Finished: 2018-03-05 11:52:23

View more information about this build on:

@wpt-pr-bot wpt-pr-bot requested review from fantasai, hakatashi, nox, plinss and r12a Mar 5, 2018

@gsnedders gsnedders merged commit ec1c5cd into web-platform-tests:master Mar 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@frivoal

This comment has been minimized.

Copy link
Contributor

frivoal commented Mar 6, 2018

It seems a bit awkward to me that the phrase "中文" which means Chinese in Chinese is tagged as being Japanese, but I guess that was that way in the original test. Since there could be differences between Chinese and Japanese line breaking, I do not suggest changing the language tag (at least not without checking with the author of the test for unintended side effects). The test is valid as merged, but maybe we could replace 中中中文 by 日日本語 or something like that.

@r12a, any thoughts?

@r12a

This comment has been minimized.

Copy link
Contributor

r12a commented Mar 6, 2018

Fwiw, i just spent the past four days (including weekend) porting the i18n test suite to github at https://github.com/w3c/i18n-tests/. I improved a bunch of tests in addition to moving them. In particular, i applied the same fix to the tests mentioned here. It may be that there are other tests in this jazh series that should have had the lang attribute added. There are separate tests for Japanese and Chinese behaviours.

@frivoal the text 中中中¢文 isn't really chinese either ;-). I chose ideographs that appear in both chinese and japanese, but didn't really try to have end-to-end meaningful strings. I also chose those han characters, iirc, because they are quite simple (and therefore easier to detect for people unlike us who know nothing of Japanese/Chinese writing).

If you think we should change, we could perhaps use something like 文字, which is both chinese and japanese. In that case, i or someone should resubmit the whole set of tests.

@r12a

This comment has been minimized.

Copy link
Contributor

r12a commented Mar 6, 2018

btw, another change i made to (all) those tests was to change

src: url('support/mplus-1p-regular.woff') format('woff');

to

src: url('../../fonts/CSSTest/mplus-1p-regular.woff') format('woff');

Compare https://github.com/mrego/web-platform-tests/blob/98b733787a32327667a1a30b7247416186aff69b/css/css-text/i18n/css3-text-line-break-jazh-059.html and https://github.com/w3c/i18n-tests/blob/gh-pages/css-text/line-breaks/css3-text-line-break-jazh-059.html

@r12a r12a referenced this pull request Mar 6, 2018

Closed

Jazh #9871

@r12a

This comment has been minimized.

Copy link
Contributor

r12a commented Mar 6, 2018

I just submitted the PR just above, which should make this one redundant.

@mrego mrego deleted the mrego:css-text-simple-fixes branch Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.