113161 - Color/background-color of preedit string is always default(we should not use invert)

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P1)

Description

[Koike Kazuhiko]

Color/background-color of preedit string is always default color.

<style>
textarea,input {
   background-color:#909090;
   color:#ffffff;
}
</style>

In textarea or input with this style, color/background-color of preedit 
string should be changed from default.

Original report in Bugzilla-jp:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=1540

Comment 1

[Roy Yokoyama]

I'm not very familiar with Linux nor Mac IME APIs. 
so cc'ing shanjian and nhotta.

Comment 2

[Frank Tang]

please put a test cases here.

Comment 3

[Koike Kazuhiko]

Attachment: Testcase

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 202062 [details] [diff] [review]
Patch rv1.0

Sorry. This patch has warning message.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.1

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This patch uses the system IME composition string color in nsTextFrame.cpp.
And this removes dirty non-XP code from |PaintTextDecorations|.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And this fixes both this and bug 170951.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The code that converts foreColor and lineColor should be factored out into its own function.

The per-IME-index values should be a single array of structs.

Other than that it looks pretty good!

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.2

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 202780 [details] [diff] [review]
Patch rv1.2

Sorry.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.2

Comment 13

[Caleb]

Off-Topic: Masayuki, this latest Bugzilla version automatically cancels pending review requests when you mark the patch as obsolete, so no need to manually remove review requests if you're going to mark it as obsolete anyway.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Caleb: Thank you.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 202781 [details] [diff] [review]
Patch rv1.2

Sorry. I don't like this patch. There is no reason for using index of array. We should use pointer for the struct instead.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.3

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.3.1

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.4

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why not just have GetIMEColor do the initialization if it hasn't been done already, instead of forcing its callers to check that?

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.5

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 203495 [details] [diff] [review]
Patch rv1.5

Oops. This may make bustage with VC6.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.6

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  if (aIMEColor->mInit)
+    return PR_TRUE;
+

Just assert !aIMEColor->mInit. The only caller guarantees this.

+nsTextPaintStyle::Get40PercentColor(nscolor aForeColor, nscolor aBackColor)

Make it a static helper function or just move it up to the caller.

+  if (!mInit && mTypes) {

mTypes is guaranteed to be non-null here. Remove the check.

+
   mInit = PR_TRUE;

Move this up inside the if block.

+  // if this retrun

"return"

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.7

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in. Thank you!