774560 - Implement text-transform: full-width

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Makoto Kato [:m_kato]]

Since full-size-kana is dropped from CSS Text Level 3 spec, full-width value is last piece of text-transform support.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: v1

Comment 2

[Simon Montagu :smontagu]

Comment on attachment 652389 [details] [diff] [review]
v1

Review of attachment 652389 [details] [diff] [review]:
-----------------------------------------------------------------

I'd like a test that covers all the affected characters. It should be no problem to generate such a test by script from http://unicode.org/Public/UNIDATA/UnicodeData.txt

I'd like to get Jonathan's opinion about this too: does this seem like the right approach or would you prefer to do it like the transforms in nsUnicodeProperties.cpp?

::: intl/unicharutil/util/nsUnicharUtils.cpp
@@ +460,5 @@
> +};
> +
> +PRUint32
> +ConvertToFullWidth(const PRUint32 aChar)
> +{

This seems to be missing U+FFE8 - U+FFEE.

Also please add a comment that we don't need to consider surrogate characters

Comment 3

[Jonathan Kew [:jfkthame]]

(In reply to Simon Montagu from comment #2)
> I'd like to get Jonathan's opinion about this too: does this seem like the
> right approach or would you prefer to do it like the transforms in
> nsUnicodeProperties.cpp?

ISTM that we should be able to extend the genUnicodePropertyData.pl tool to extract the fullwidth mappings from UnicodeData.txt, and use a two-stage lookup table like those we're using for other character properties. That ought to provide more efficient lookup, particularly for the (many) CJK characters that are _not_ affected by the transform.

As currently written, if the user applies text-transform:full-width to a block of text that may contain some ASCII or kana, but is mostly Kanji, it looks like the ConvertToFullWidth() function will end up doing about a dozen "if"-tests for almost every character. The alternative approach would reduce that to a single "if" and a couple of array-indexing calculations. Less branching is probably better, in general.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: pt 1 - add Unicode full-width character mapping data.

This extends our Unicode property data to support fullwidth mapping info, based on the <wide> and <narrow> compatibility decompositions in the Unicode database.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: pt 2 - implement text-transform:full-width.

This is basically m_kato's patch, just rebased to current trunk and modified to use the fullwidth mapping from the preceding patch instead of a custom function.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: pt 3 - testcase for all characters with a full-width mapping.

Testcase covering all the characters for which text-transform:full-width has a mapping.

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 678130 [details] [diff] [review]
> pt 2 - implement text-transform:full-width.
> 
> This is basically m_kato's patch, just rebased to current trunk and modified
> to use the fullwidth mapping from the preceding patch instead of a custom
> function.

Oh, one more thing we should do in this patch is to add "full-width" to the other_values for text-transform in layout/style/test/property_database.js.

Comment 8

[Jonathan Kew [:jfkthame]]

Tryserver run: https://tbpl.mozilla.org/?tree=Try&rev=2c246ba76534

Comment 9

[Simon Montagu :smontagu]

Comment on attachment 678130 [details] [diff] [review]
pt 2 - implement text-transform:full-width.

Review of attachment 678130 [details] [diff] [review]:
-----------------------------------------------------------------

I take it we're OK with implementing this unprefixed?

Comment 10

[Jonathan Kew [:jfkthame]]

Hmm... I assumed so, but let's check with dbaron.

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 678130 [details] [diff] [review]
pt 2 - implement text-transform:full-width.

fantasai, what's the current status of text-transform:full-width in CSS3 Text - in particular, should we be implementing it unprefixed at this point?

Comment 12

[Jonathan Kew [:jfkthame]]

from #layout, 2012-11-08:
[8:25pm] jfkthame: dbaron_: so did fantasai's reply re css3-text sound stable enough for us to land full-width without prefixing it?
[9:32pm] dbaron_ is now known as dbaron.
[9:32pm] dbaron: jfkthame, yes, I think so, assuming that we track the spec closely until it gets to CR
[9:33pm] jfkthame: dbaron: ok, sounds good to me
[9:34pm] jfkthame: dbaron: it'll take awhile to work its way up to the release channel anyway
[9:35pm] dbaron: jfkthame, yeah, that's basically what I was thinking
[9:35pm] jfkthame: right - so if something comes up in the next couple months, we could still hold back

After checking with fantasai & dbaron (email and irc), we'll go ahead with this. We'll keep an eye on the spec as it heads towards CR (editors are aiming for Last Call by the end of the year), just in case anything comes up that would cause us to re-evaluate before this hits the release channel.

Comment 13

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9153a503df43
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ca78bd4d6da
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b84789d3b81

(m_kato, apologies for "stealing" your bug, and thanks for getting it started!)

Comment 14

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/9153a503df43
https://hg.mozilla.org/mozilla-central/rev/6ca78bd4d6da
https://hg.mozilla.org/mozilla-central/rev/9b84789d3b81

Comment 15

[Jean-Yves Perrier [:teoli]]

I've documented it in:
https://developer.mozilla.org/en-US/docs/CSS/text-transform
and
https://developer.mozilla.org/en-US/docs/Firefox_19_for_developers

Two points I note:
1) we can't mixed full-width and uppercase in CSS Text L3 (and in the implementation) but we would be able to do in CSS Text L4.
2) Only latin characters without diacritics are affected (é à ö and other å stay narrow). (This is a Unicode constraint if I have read correctly)

Am I correct?

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to Jean-Yves Perrier [:teoli] from comment #15)
> Two points I note:
> 1) we can't mixed full-width and uppercase in CSS Text L3 (and in the
> implementation)

That's correct...

> but we would be able to do in CSS Text L4.

...and so is that, according to the current draft, though obviously that's subject to possible change.

> 2) Only latin characters without diacritics are affected (é à ö and other å
> stay narrow). (This is a Unicode constraint if I have read correctly)

Yes, only a very limited set of letters (and a few symbols, etc) have fullwidth variants. Note that it doesn't _only_ apply to Latin characters, however; there are also a bunch of "halfwidth" Japanese kana and Korean hangul letters in Unicode that map to fullwidth versions.

(You can see the full collection of affected characters in the test file layout/reftests/text-transform/fullwidth-all.html.)