74123 - in Arabic/Hebrew/Indic/Thai keyboard, mozilla assert a lot

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P2)

Description

[Frank Tang]

If I switch to Arabic/Hebrew/Indic/Thai keyboard and type space, mozilla will 
assert because it will convert the space into 3 unicode. a RLO + space + PDF

In that case, we should ignore the RLO and PDF.

Comment 1

[Frank Tang]

Attachment: patch

Comment 2

[Frank Tang]

not a perfect solution. We can still only return one keycode here. (We should be 
able to generate n keypress event for this one)
But this patch will reduce the chance we hit assert. 

Comment 3

[Kathleen :Brade]

Please fix the tab to be spaces.
Please correct typo in first printf in debug: "on" --> "one"
Please correct typo in last assertion in diff:  "ConverFrom" --> "ConvertFrom"
Is there another bug filed for the case we don't handle correctly (more than one 
unicode)?  If so, it might be good to put that bug # in the assertion or in the 
comments near where it says "fix me"

r=brade

Comment 4

[Simon Fraser [no longer active]]

I can't imagine that the proper solution for the case where we want to do more 
than one keypress is hard. This method could just return an array of PRUint32, 
and a number, like:

// outChars allocated by the caller, probably on stack
// ioNumChars -> on input, max number of slots in outChars
//               on output, number of outChars filled
void nsMacEventHandler::ConvertKeyEventToUnicode(EventRecord& aOSEvent, PRUint32 
outChars[], PRInt32 &ioNumChars)

Then the calling routine can just iterate through the values passed back.

Comment 5

[Frank Tang]

simon-
The problem is the caller is currently  nsMacEventHandler::InitializeKeyEvent and 
it's caller is nsMacEventHandler::HandleKeyEvent. and 
nsMacEventHandler::HandleKeyEvent is the one which call 
DispatchWindowEvent(keyEvent);

It is not "that hard" to fix it. But it at least require me to change at least 3 
functions at once to make it work (HandleKeyEvent, InitializeKeyEvent, and 
ConvertKeyEventToUnicode) . I feel very very nervous to change 
InitializeKeyEvent. It is not hard to change ConvertKeyEventToUnicode  and 
HandleKeyEvent at all)

I suggest we take a step-by-step approach. Check in this part to make the most 
cases working first. and rewrite that part a little bit later. 

brade- what do you think ?
 

Comment 6

[Frank Tang]

still need to talk to sfraser about this issue. move to moz0.9.1

Comment 7

[Andreas Becker]

Nominating for nsbeta1.

Comment 8

[Frank Tang]

nhotta- I am overload . can you help drive this one ?

Comment 9

[nhottanscp]

Conversion Control Flags:
http://developer.apple.com/techpubs/macos8/TextIntlSvcs/TextEncodingConversionMa
nager/TEC1.5/TEC.6d.html

Conversion Control Masks:
http://developer.apple.com/techpubs/macosx/Carbon/text/TextEncodConversionMgr/Te
xt_Encodin_sion_Manager/Enumerations/Conversion_Control_Masks.html

Comment 10

[nhottanscp]

Move to 0.9.2 and reassign to ftang.

Comment 11

[Frank Tang]

remove moz0.9.2

Comment 12

[Jaime Rodriguez, Jr.]

Ftang - Are you saying we don't need this one for M0.9.2?

Comment 13

[Jaime Rodriguez, Jr.]

Marking as nsbeta1-, as we have not set a milestone for this one.

Comment 14

[Jaime Rodriguez, Jr.]

Setting to M0.9.2 | P2 for now. Ftang, pls change, if you think this is 
inappropriate.

Comment 15

[Frank Tang]

we don't have time to work on Mac BIDI now . remove moz0.9.2

Comment 16

[Frank Tang]

move to future. No time to work on Mac Arabic now.

Comment 17

[Frank Tang]

this problem can be addressed by patch in bug 77038

Comment 18

[Simon Montagu :smontagu]

ftang: now that bug 77038 is marked as fixed, can you revisit this one and
confirm whether the patch there fixed this issue also?

Comment 19

[Roy Yokoyama]

ylong: can you verify this bug being fixed by 77038?

Comment 20

[Yuying Long]

I just checked it with Hebrew keyboard layout, seems work fine.

Comment 21

[Frank Tang]

mark it as fixed

Comment 22

[Yuying Long]

Mark as verified.