307396 - Add IME selection colors and Underline height (nsILookAndFeel)

Status: RESOLVED FIXED

(Core :: Widget, defect, P1)

Description

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

I need IME selection colors and Underline height for bug 113161 and bug 170951.
I will attach the patch.

Comment 1

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

Attachment: Patch rv1.0

Comment 2

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

Attachment: Patch rv1.0 (widget/public, widget/src/xpwidgets)

We need following status's foreground and background colors:
See
http://lxr.mozilla.org/seamonkey/source/content/base/public/nsISelectionController.idl#57


nsISelectionController::SELECTION_IME_SELECTEDRAWTEXT
nsISelectionController::SELECTION_IME_SELECTEDCONVERTEDTEXT
nsISelectionController::SELECTION_IME_RAWINPUT
nsISelectionController::SELECTION_IME_CONVERTEDTEXT

And underline's height magnification.
See http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#2142


2142 #ifdef XP_MACOSX // underline thicness is 2 pixel
2143   aRenderingContext.FillRect(aX + startOffset+size, aY + baseline -
offset, textWidth-2*size, 2*size);
2144 #else
2145  aRenderingContext.FillRect(aX + startOffset+size, aY + baseline - offset,
textWidth-2*size, size);
2146 #endif

Comment 3

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

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

-const char nsXPLookAndFeel::sColorPrefs[][36] =
+const char nsXPLookAndFeel::sColorPrefs[][38] =

You want 48, not 38. I'm not show how this compiled.

Comment 4

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

(In reply to comment #3)
> (From update of attachment 195171 [details] [diff] [review] [edit])
> -const char nsXPLookAndFeel::sColorPrefs[][36] =
> +const char nsXPLookAndFeel::sColorPrefs[][38] =
> 
> You want 48, not 38. I'm not show how this compiled.
> 

Why 48? Most long member is "ui.IMESelectedConvertedTextBackground". This is 37
charcters.

Comment 5

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

O.K. I understand. You may understand that the index is prefs count. It's not
so. It is string length of each items. 38 is enough for current patch.

Therefore, I checked-in.

-> FIXED

Comment 6

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

Attachment: Patch for check-in

Comment 7

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

You're right. That was my mistake.

I'm not sure why this code is like that. Couldn't it be written

const char* nsXPLookAndFeel::sColorPrefs[] = { ... }?

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #7)
> I'm not sure why this code is like that. Couldn't it be written
> 
> const char* nsXPLookAndFeel::sColorPrefs[] = { ... }?

that's less efficient per http://people.redhat.com/drepper/dsohowto.pdf 2.4

Comment 9

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

True. I hate ELF.