202208 - text--transform: capitalize incorrectly always places the first character in titlecase

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Ernest Cline]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3) Gecko/20030312

Capitalize should only capitalize the first letter. Mozilla currently changes
the first character to titlecase all the time. This causes problems for Mozilla
when the first character is a character composed of two uppercase letters, such
as U+0191 'DZ'.

Reproducible: Always

Steps to Reproduce:
1. Apply text-transform:capitalize to an uppercase character which is not the
same as its titlecase character.

Actual Results:  
The uppercase character is transformed to titlecase (In the given example U+0192.)

Expected Results:  
The uppercase should have remained in uppercase. The algorithm thatshould be
used is if the first character is not in uppercase, transform it into titlecase.

Comment 1

[Ernest Cline]

Attachment: Testcase showing the problem

Comment 2

[Boris Zbarsky [:bzbarsky]]

intl.

Comment 3

[Simon Montagu :smontagu]

Removing dependency. I've decided to keep this bugfix separate from bug 210501

Comment 4

[Simon Montagu :smontagu]

Attachment: Patch

I also changed the expected results in UnicharSelfTest.cpp to fit the change,
and since I was there, added a test for ToTitle(PRUnichar*, PRUnichar*,
PRUint32). This exposed a nasty bug (previously unnoticed because all other
callers transform the buffer in place) which I have also fixed.

Comment 5

[Jungshik Shin]

Comment on attachment 178727 [details] [diff] [review]
Patch

>+  // First check for uppercase characters whose titlecase mapping is different,
>+  // like U+01F1 DZ: they must remain unchanged.
>+  if( 0x01C0 == ( aChar & 0xFFC0)) // 0x01Cx - 0x01Fx
  ... snip ..

How about moving this block after the ASCII case? 

>   if( IS_ASCII(aChar)) // optimize for ASCII
>   {


>+     for (i = 0; i < T4LEN; i++)

  Although it works as it is now, |i+=2| is better because the input chunk size
is 2 below.

>+     {
>+       PRUnichar* titleTest = t4data + i;
>+       res = t->ToTitle(titleTest, buf, 2);
>+       if(NS_FAILED(res)) {
>+         printf("\tFailed!! return value != NS_OK\n");
>+       } else {
>+         if (buf[0] != t4result[i] || buf[1] != t4data[i + 1])

|t4data[i + 1]| should be |t4result[i + 1]|, shouldn't it? 

With those changes, r=jshin

Comment 6

[Simon Montagu :smontagu]

(In reply to comment #5)
>   Although it works as it is now, |i+=2| is better because the input chunk size
> is 2 below.


> >+         if (buf[0] != t4result[i] || buf[1] != t4data[i + 1])
> 
> |t4data[i + 1]| should be |t4result[i + 1]|, shouldn't it? 

No: what I'm trying to do here is pass a series of buffers each 2 characters
long, using every position in the test data as starting character, and then
check that ToTitle() transforms the first character as expected and leaves the
second character as is. I'll add comments to make this clear.

Comment 7

[Jungshik Shin]

Aha. Sorry I forgot that it's 'Titlecase' !!

Comment 8

[Simon Montagu :smontagu]

Attachment: Made the first change Jungshik suggested and added comments

Transferring r=jshin and requesting sr.

Comment 9

[Simon Montagu :smontagu]

Grr. I just saw a typo: "expect" for "expected". I won't attach a new patch, but
I've already corrected that locally.

Comment 10

[rbs]

Comment on attachment 178784 [details] [diff] [review]
Made the first change Jungshik suggested and added comments

r=rbs

Comment 11

[Simon Montagu :smontagu]

Checked in.