313174 - IME composition string isn't rendered as selection text

Status: RESOLVED FIXED

Description

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

Bug-org 242315 is fixed. But it may have a regression.
On Be-OS, we don't render the composition string as selection text.
This is serious regression.

Comment 1

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

Bug-org 242315 -> Bug 242315

Comment 2

[Sergei Dolgov]

fix is simple, adding || defined(XP_BEOS), as BeOS supports InvertRect

Comment 3

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

Created attachment 200247 [details] [diff] [review]
Patch rv1.0

Maybe, fix.

Comment 4

[Sergei Dolgov]

i can review patch from BeOS side - as it worked for me and Koki-san at selection.
But does it need sr ?

Comment 5

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

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

> But does it need sr ?

Yes.

Comment 6

[Sergei Dolgov]

How it looks in BeOS with patch applied:
http://beos.spb.ru/mozilla/BeIMEInvertRect.png

Comment 7

[Sergei Dolgov]

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

r=sergei_d

Comment 8

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

looks good.

Comment 9

[Sergei Dolgov]

sorry, forget to set Haru font for screenshot - problem with Harus is that it
produces wrong spacing (double) for cyrillic letters (my default locale is
Russian) in BeOS. But result is understandable anyway

Comment 10

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

I can see the selection rendering. It's enough. Thanks.

Comment 11

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

Boris:

Could you superreview to this?
This patch is very simple and this is blocker bug for BeOS user.
# I tried to contact to roc, but I don't have his reply...

Comment 12

[Boris Zbarsky [:bzbarsky, bz on IRC]]

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

sr=bzbarsky, though I have some questions:

1)  If it's a blocker, why wasn't blocking? requested?  The chance of getting this into 1.8 is about 0 at this point.

2)  Why do we need this whitelist of XP* stuff?  Why not just USE_INVERT_RECT on anything that's not Mac?

Comment 13

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

> 1
Bug 242315 is not going to 1.8 branch.

> 2
Mac is changed by bug 236996.
# And I will rewrite these code in 1.9 cycle.

Comment 14

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

checked-in to Trunk.

Sergei:

If you need approval for this, you should request it.

Comment 15

[Sergei Dolgov]

Niels has his own plans about 1.8* - he decided to skip RC1 as far as i know.
Usually it is his job/privilege to set approval requests for 1.8(FF 1.5).

Comment 16

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Just so people are clear, RC1 is a real RC.  That's the code we'll ship unless something comes up that's stop-ship.

Comment 17

[Niels Reedijk]

It's clear, however, I sort of hope that (In reply to comment #16)
> Just so people are clear, RC1 is a real RC.  That's the code we'll ship unless
> something comes up that's stop-ship.

I'm sort of hoping that that would be the case. Anyway, else I'll just release a build with custom patches, so it's not a major problem.