[FEATURE]Enable Unicode on TSM1.5

RESOLVED FIXED in mozilla0.9.6

Status

()

Core
Internationalization
P3
normal
RESOLVED FIXED
18 years ago
13 years ago

People

(Reporter: Frank Tang, Assigned: Frank Tang)

Tracking

({helpwanted})

Trunk
mozilla0.9.6
PowerPC
Mac System 8.6
helpwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 13 obsolete attachments)

2.11 KB, patch
Kathleen Brade
: review+
Details | Diff | Splinter Review
2.89 KB, text/plain
Kathleen Brade
: review+
Details
41.40 KB, patch
Details | Diff | Splinter Review
43.75 KB, patch
Simon Fraser
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

18 years ago
In order to fully implement the TSM 1.5 shipped w/ MacOS 8.5 and later and the
Unicode keyboard layout shipped with MacOS 9. We need to do the following
1. Check MacOS support TSM 1.5 or not
2. If it did, identify itself can handle unicode by adding kUnicodeDocument
'udoc' to the
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacEventHandler.cpp#244

243   supportedServices[0] = kTextService;
244   err = ::NewTSMDocument(1,supportedServices,&mTSMDocument,(long)this);

We need to make sure supportedServices are > 2 and
add supportedServices[1] = kUnicodeDocument;
in that case

3. We need to add code to handle Unicode input in the current TSM code.
Howerver, we need to make sure MacOS which do not have TSM 1.5 still work.
4. We need to add additional AppleEvent Handler for kUnicodeNotFromInputMethod
event
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Whiteboard: [Help Wanted]
Target Milestone: M20
(Assignee)

Comment 1

18 years ago
Not a beta feature, Mark it M20., Anyone from the net want to help ??

Updated

18 years ago
Keywords: helpwanted

Updated

18 years ago
Whiteboard: [Help Wanted]

Updated

18 years ago
QA Contact: teruko → ftang
(Assignee)

Comment 2

17 years ago
mark it as future.
Target Milestone: M20 → Future
(Assignee)

Comment 3

17 years ago
IME related issue. REassign to yokoyama
Assignee: ftang → yokoyama
Status: ASSIGNED → NEW

Comment 4

17 years ago
They are two open source editor that support this on the Mac :
MLTE demo and SUE.

Both of them have been written more as small demonstration of this feature than
anything else.

The autor of MLTE explicitly states :  "Full source code included and freely
reusable in other projects".

Therefore I think it would be possible to reuse some of this code to help
implement this in Mozilla, or from SUE that has a similar philosophy.

Both are described from this page that might be more explicit about their
features that the direct access one :
http://www.hclrss.demon.co.uk/unicode/utilities_editors.html

Direct access with :
http://www.merzwaren.com/snippets/index.html#mltedemo
http://members.tripod.com/%7Etomaszek/sue.html

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 5

17 years ago
Mac issue: assigning to nhotta@netscape.com
Assignee: yokoyama → nhotta
Status: ASSIGNED → NEW

Comment 6

16 years ago
Reassign to ftang.
Assignee: nhotta → ftang
(Assignee)

Comment 7

16 years ago
mark it as assigned
Status: NEW → ASSIGNED
(Assignee)

Comment 8

16 years ago
Finally I have time to hack it.
I want to break down to two stages.
Statge one- upgrade the Input Method code to use Unicode of TSM 1.5 or higher is 
present.
Stage two- add kUnicodeNotFromInputMethod handler to support Unicode Keyboard. 

Here is the patch for the first step, Test on MacOS9.1, have not test on MacOSX 
yet.
Target Milestone: Future → mozilla0.9.4
(Assignee)

Comment 9

16 years ago
Created attachment 45345 [details] [diff] [review]
patch to migrate to Unicode Input Method on TSM1.5
(Assignee)

Updated

16 years ago
Blocks: 77038
(Assignee)

Comment 10

16 years ago
try the latest patch on MacOS X. No problem with Japanese input (do not have 
Korean/chinese input method on MacOSX yet). 
The candidcte window some times show in wrong places. We probably need to fix 
that later by changing the kOffset2Pos and kPos2Offset handler. 

Please code review so I can land this and move to working on 77038.

Change this summary from "[FEATURE]Enable Unicode keyboard layout on TSM1.5" to
"[FEATURE]Enable Unicode on TSM1.5"

let this bug focus on Unicode Input Method, and let bug 77038 become the Unicode 
Keyboard Layout bug. We need to fix this first before adding Unicode 
kUnicodeNotFromInputMethod handler . 
Summary: [FEATURE]Enable Unicode keyboard layout on TSM1.5 → [FEATURE]Enable Unicode on TSM1.5

Comment 11

16 years ago
Comments on the patch:

* All new code: please follow Mozilla's spacing conventions better. Spaces
  after 'if', spaces between argument parameters etc.

* Changes to nsMacTSMMessagePump.h/cpp look fine (apart from non-standard
  spacing and \ No newline at end of file)

* I think I'd like to see the TSM-related code in nsMacEventHandler.cpp
  move into a new file. You added a lot of new code that is IME-specific,
  and it seems that it would make sense in its own file.

* You left debug output on:

-//#define DEBUG_TSM
+#define DEBUG_TSM

I didn't check the logic in UnicodeHandleUpdateInputArea; i'll leave the reviewer 
to do that. But I did notice:

+               if(NS_FAILED(res)) 
+                       goto error; 

You'll leak in a number of places with an early return like this. Would be better 
to use auto_ptr or something to ensure cleanup.

+               if(xpTextRangeArray) 
+                       delete [] xpTextRangeArray;

This if() test is redundant for two reasons: 1) you know that if you got there, 
xpTextRangeArray is never non-null, and 2) even if it was, |delete| and |delete 
[]| are null-safe.

Another one:
\ No newline at end of file
(Assignee)

Comment 12

16 years ago
Created attachment 46013 [details]
widget/src/mac/nsTSMStrategy.cpp
(Assignee)

Comment 13

16 years ago
Created attachment 46014 [details]
widget/src/mac/nsTSMStrategy.h
(Assignee)

Comment 14

16 years ago
Created attachment 46015 [details] [diff] [review]
patch v2 include keyboard stuff
(Assignee)

Comment 15

16 years ago
OK, the new patch include two new file widget/src/mac/nsTSMStrategy.h and widget/
src/mac/nsTSMStrategy.cpp  

and it also including the code to provide Unicode base keyboard code. 
(Assignee)

Comment 16

16 years ago
not sure which milastone I want to land this. brade@netscape.com, can you r=
this one?
Maybe we should wait till m94 branch off. 
Target Milestone: mozilla0.9.4 → ---

Comment 17

16 years ago
Sorry; I ran out of time before I could complete the code review.  I hope this is 
useful feedback/comments.

The two new files look fine except they appear to have tabs or a large number of 
spaces instead of the mozilla standard of 2 spaces.  Could you change these to 
use two spaces for each level of indenting?

Formatting issues:
-----------------
* In nsMacTSMMessagePump.cpp, search for your changes with "if(" and change to 
"if ("
* Also, nsMacTSMMessagePump::UnicodeUpdateHandler seems to have tabs instead of 2 
spaces per indentation level.
* I find it easier to read parameters if there is a space after the ',' 
especially if there are other spaces like this partial line:
(unicodeTextPtr,text_size/2,fixLength / 2,hiliteRangePtr)
should be:
(unicodeTextPtr, text_size/2, fixLength / 2, hiliteRangePtr)
* Also, note the spacing inconsistency above with "text_size/2" & "fixLength / 2"
* Fix tabs/spaces issues in nsMacEventHandler.cpp
* in nsMacEventHandler::InitializeKeyEvent, indent additional parameters over to 
be under nsKeyEvent& aKeyEvent
* also in InitializeKeyEvent, would like to see new parameters start with 'a' so 
we'd have aIsChar and aConvertChar
* Add a blank line before the comment for HandleUKeyEvent

Code issues:
-----------
* In nsMacTSMMessagePump::UnicodeUpdateHandler, why does err4 condition return 
noErr instead of the error encountered from the AE call?
* In nsMacTSMMessagePump::UnicodeUpdateHandler, inside an #if TARGET_CARBON, you 
have "nsAutoString unicodeText;" in both the #if and the #else, it could be 
pulled outside of the #if/#else.
* Variable "actualType" is not used in 
nsMacTSMMessagePump::UnicodeNotFromInputMethodHandler
* It might help cleanup UnicodeUpdateHandler and UnicodeNotFromInputMethodHandler 
if they had a helper function to grab the event handler and the unicode text.  
Could this helper method also do the memcopy to the nsAutoString "unicodeText" or 
could that be moved to another helper?
* In nsMacEventHandler.cpp, +#define DEBUG_TSM should not be changed (should not 
be set to true).

Other issues:
------------
* Has anyone checked for memory leaks?  Leaking 2 bytes per keystroke while in 
IME can cause HUGE problems (as Kat Momoi found back in 4.0 version).
* Also, I would feel more comfortable if Kat, Teruko, or someone else also tested 
this for 15-20 minutes of solid typing, since (from what I've seen), different 
IME users have different styles for inputting.  Some people invoke the list more 
frequently or make corrections in different ways (arrows vs backspace).
* If several people have tested on both OS9 and OSX and found no major issues 
(regressions) and we are confident we have no memory leaks in this area, I am ok 
with checking in sooner (rather than waiting until after 0.9.4).
(Assignee)

Comment 18

16 years ago
shoot for m94
Target Milestone: --- → mozilla0.9.4

Updated

16 years ago
Keywords: nsenterprise+
(Assignee)

Comment 19

16 years ago
tiantian: do we really need this for enterprise ? I don't think we can make it
for m0.9.4 . We can still type in Japanese/Chinese/Korean /French/German without
fixint this one. We just cannot type vietnamese....

move to m0.9.6. Please let me know if you really really need it for m0.9.4
Target Milestone: mozilla0.9.4 → mozilla0.9.6

Comment 20

16 years ago
marking nsenterprise-
Keywords: nsenterprise+ → nsenterprise-
(Assignee)

Updated

16 years ago
Blocks: 103669
(Assignee)

Comment 21

16 years ago
Created attachment 52740 [details]
v2 of mozilla:widget:src:mac:nsTSMStrategy.cpp which address brade's comment
(Assignee)

Comment 22

16 years ago
Created attachment 52741 [details]
v2 of mozilla:widget:src:mac:nsTSMStrategy.h which address brade's comment
(Assignee)

Updated

16 years ago
Attachment #52740 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #52741 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #45345 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #46013 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #46014 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #46015 - Attachment is obsolete: true
(Assignee)

Comment 23

16 years ago
Created attachment 52758 [details] [diff] [review]
v3 of mozilla:widget:src:mac:nsTSMStrategy.cpp addressed brade's comment
(Assignee)

Comment 24

16 years ago
Created attachment 52759 [details] [diff] [review]
v3 of mozilla:widget:src:mac:nsTSMStrategy.h addressed brade's comment
(Assignee)

Comment 25

16 years ago
Created attachment 52778 [details] [diff] [review]
v3 of patch for other files, address brade's comment. Without ignore space
(Assignee)

Comment 26

16 years ago
Created attachment 52779 [details] [diff] [review]
same patch as above (v3) except ignore space (-b) in cvs diff
(Assignee)

Updated

16 years ago
Blocks: 104056

Comment 27

16 years ago
Comment on attachment 52758 [details] [diff] [review]
v3 of mozilla:widget:src:mac:nsTSMStrategy.cpp addressed brade's comment

r=brade with whitespace changes we discussed
Attachment #52758 - Flags: review+
(Assignee)

Comment 28

16 years ago
Comment on attachment 52758 [details] [diff] [review]
v3 of mozilla:widget:src:mac:nsTSMStrategy.cpp addressed brade's comment

submit new patch
Attachment #52758 - Attachment is obsolete: true

Comment 29

16 years ago
Comment on attachment 52759 [details] [diff] [review]
v3 of mozilla:widget:src:mac:nsTSMStrategy.h addressed brade's comment

r=brade if you add a blank line before the class definition
Attachment #52759 - Flags: review+
(Assignee)

Comment 30

16 years ago
Created attachment 52919 [details]
v4 of mozilla:widget:src:mac:nsTSMStrategy.cpp

Comment 31

16 years ago
Comment on attachment 52919 [details]
v4 of mozilla:widget:src:mac:nsTSMStrategy.cpp

r=brade
Attachment #52919 - Flags: review+

Comment 32

16 years ago
in nsMacEventHandler.cpp, 
 * add blank lines before "static PRBool gUseUnicodeAPI = PR_FALSE;"
   and after "gInitUseUnicodeAPI = PR_FALSE;"
 * change this line:
     if(tsmstrategy.UseUnicodeForInputMethod()) {
     if (tsmstrategy.UseUnicodeForInputMethod()) {
 * change this line:
     err = ::NewTSMDocument(1,supportedServices,&mTSMDocument,(long)this);
     err = ::NewTSMDocument(1, supportedServices, &mTSMDocument, (long)this);
 * change this line:
     mIMECompositionStr=nsnull;
     mIMECompositionStr = nsnull;
 * change this block:
     aKeyEvent.eventStructType = NS_KEY_EVENT;
     aKeyEvent.message     = aMessage;
     aKeyEvent.point.x      = 0;
     aKeyEvent.point.y      = 0;
     aKeyEvent.time        = PR_IntervalNow();
   to something like:
     aKeyEvent.eventStructType = NS_KEY_EVENT;
     aKeyEvent.message = aMessage;
     aKeyEvent.point.x = 0;
     aKeyEvent.point.y = 0;
     aKeyEvent.time = PR_IntervalNow();
   Do the same with this code (changes already made):
     aKeyEvent.widget = aFocusedWidget;
     aKeyEvent.nativeMsg = (void*)&aOSEvent;
   The modifier flags assignments can stay as they are.
 * Change this line (note removal of space after ';' too):
     *aIsChar =  PR_FALSE;
     *aIsChar = PR_FALSE;
 * Change this line:
     if ( aKeyEvent.isControl )
     if (aKeyEvent.isControl)
 * Change this line:
     ( aKeyEvent.charCode <= 26 )
     (aKeyEvent.charCode <= 26)
 * Change this line:
     ( aKeyEvent.isShift )
     (aKeyEvent.isShift)
 * Change this line:
     } // if ( aKeyEvent.charCode <= 26 )
     } // if (aKeyEvent.charCode <= 26)
 * Change this block:
       aKeyEvent.keyCode  = 0;
     } // if ( aKeyEvent.isControl )
     else // else for if ( aKeyEvent.isControl )
   to:
       aKeyEvent.keyCode = 0;
     } // if (aKeyEvent.isControl)
     else // else for if (aKeyEvent.isControl)
 * Change this line:
     if ( !aKeyEvent.isMeta)
     if (!aKeyEvent.isMeta)
 * Change this line:
     } // if ( !aKeyEvent.isMeta)
     } // if (!aKeyEvent.isMeta)
 * Change this line to 2 lines (to be more consistent with rest of that method):
     if (aConvertChar) {
     if (aConvertChar)
     {
 * Move the "{" down for the line that begins "if (aKeyEvent.isShift" also
 * Change this line:
     } // else for if ( aKeyEvent.isControl )
     } // else for if (aKeyEvent.isControl)
 * Change this line (in HandleUKeyEvent):
     nsKeyEvent  keyEvent;
     nsKeyEvent keyEvent;
 * Change this line:
     InitializeKeyEvent(keyEvent,aOSEvent, focusedWidget,NS_KEY_PRESS, ...
     InitializeKeyEvent(keyEvent, aOSEvent, focusedWidget, NS_KEY_PRESS, ...
 * Move this line above "PRUint32 i;" and fix typo in "unicode":
     // it is a message with text, send all the unciode characters
     // it is a message with text, send all the unicode characters
 * Insert a blank line after the HandleUKeyEvent method (before the #pragma)
 * Change this line:
     nsresult nsMacEventHandler::UnicodeHandleUpdateInputArea(PRUnichar* 
text,long charCount, long fixedLength,TextRangeArray* textRangeList)
   to these lines (or similar; be sure to add space after ','):
     nsresult nsMacEventHandler::UnicodeHandleUpdateInputArea(PRUnichar* text,
                                          long charCount, long fixedLength,
                                          TextRangeArray* textRangeList)
 * Change this line and move it down to the 2.2.2 section (where it's used):
     int          i;
     int i;
 * Every place you see this should be changed:
     if(NS_FAILED(res))
     if (NS_FAILED(res))
 * Change this line:
     if(nsnull == mIMECompositionStr)
     if (nsnull == mIMECompositionStr)

More in a little bit...

Comment 33

16 years ago
continuing...
 * Change this line (partial:  in->at and typo):
     in the same time. The easies way
     at the same time. The easiest way
 * Change this line:
     if(0 != committedLen)
     if (0 != committedLen)
 * Change this line:
     if((-1 != fixedLength) && (charCount != fixedLength ))
     if ((-1 != fixedLength) && (charCount != fixedLength))
 * Change this line (typo and missing space):
     printf("Have new uncommited text from %d to text_size(%d)\n", 
committedLen,charCount);
     printf("Have new uncommitted text from %d to text_size(%d)\n", committedLen, 
charCount);
 * Change this line (typo):
     // 2.1 send compositionEvent to start the comosition
     // 2.1 send compositionEvent to start the composition
 * Change this line (typo):
     // if we aren't in composition mode alredy, signal the backing store w/ ...
     // if we aren't in composition mode already, signal the backing store w/ ...
 * Change this line (typo):
     }   // 2.1 send compositionEvent to start the comosition
     }   // 2.1 send compositionEvent to start the composition
 * Remove extra spaces (2) from this line:
     nsTextRangeArray  xpTextRangeArray  = new nsTextRange[ ...
     nsTextRangeArray xpTextRangeArray = new nsTextRange[ ...
 * Add spaces (4) to this line and remove extra spaces from the end of the line:
     for(i=0; i<rangeArray->fNumOfRanges; i++) {
     for(i = 0; i < rangeArray->fNumOfRanges; i++) {
 * Remove this line; it shouldn't be needed (per sfraser's comments above):
     if(xpTextRangeArray)
   Fix indent of delete [] line (of course).
 * Add space on this line:
     else if((0==charCount) && (0==fixedLength))
     else if ((0==charCount) && (0==fixedLength))
 * Fix typo on this line (un->up):
     In that case, we need to send a text event to clean un the input hole....
     In that case, we need to send a text event to clean up the input hole....
 * Add space on this line:
     res = HandleTextEvent(0,nsnull);
     res = HandleTextEvent(0, nsnull);
 * Add blank line before this line:
     error:
 * Is there really no cleanup to do in any of the error conditions?  If so, why 
not follow convention and just "return res;" instead of having a goto?


In nsMacTSMMessagePump.cpp,
 * The spacing doesn't look correct on these 2 lines (are there tabs?):
    mUpdateUPP = NewAEEventHandlerUPP(nsMacTSMMessagePump::UnicodeUpdateHandler);
    mUpdateUPP = NewAEEventHandlerUPP(nsMacTSMMessagePump::UpdateHandler);
    mKeyboardUPP = NewAEEventHandlerUPP(nsMacTSMMessagePump::UnicodeNotFrom ...
    err = AEInstallEventHandler(kTextServiceClass, kUnicodeNotFromInputMethod ...
    NS_ASSERTION(err==noErr, "nsMacTSMMessagePump::InstallTSMAEHandlers: ...
    err = AERemoveEventHandler(kTextServiceClass, kUnicodeNotFromInputMethod ...
    NS_ASSERTION(err==noErr, "nsMacTSMMessagePump::InstallTSMAEHandlers: ...
    ::DisposeAEEventHandlerUPP(mKeyboardUPP);
 * After this line:
    ::DisposeAEEventHandlerUPP(mUpdateUPP);
   There is a blank line with some spaces on it; remove the spaces.

more in a little while...

Comment 34

16 years ago
in nsMacTSMMessagePump.cpp,
 * in AETextToString, you should use SetLength instead of SetCapacity (according 
to scc)
 * check for tabs in this file; do a search/replace by copying/pasting the tab 
into the find edit field of the dialog.
 * Remove extra space (2 different lines):
     EventRecord         event ;
     EventRecord         event;
 * Change these lines (remove extra spaces):
     err = AEGetParamPtr ( theAppleEvent, keyAETSMEventRecord, 
typeLowLevelEventRecord, & returnedType,
         & event, sizeof ( event ), & actualSize );
   to:
     err = AEGetParamPtr(theAppleEvent, keyAETSMEventRecord, 
typeLowLevelEventRecord, 
         &returnedType, &event, sizeof(event), &actualSize);
 * Check for the result on this line:
     AETextToString(text, unicodeText, text_size);

more in a little while... (sorry I have so many interruptions!)

Comment 35

16 years ago
Created attachment 52965 [details]
code snippet of some factoring that might be done (with other cleanup as well)
(Assignee)

Comment 36

16 years ago
Comment on attachment 52778 [details] [diff] [review]
v3 of patch for other files, address brade's comment. Without ignore space

see the new patch
Attachment #52778 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #52779 - Attachment is obsolete: true
(Assignee)

Comment 37

16 years ago
Created attachment 53036 [details] [diff] [review]
v4 of the diff of existing files
(Assignee)

Comment 38

16 years ago
Created attachment 53037 [details] [diff] [review]
v4 of the diff of existing files igore space changes (with -b in cvs diff)
(Assignee)

Comment 39

16 years ago
The last two attached diff include all the comment brade gave previously
one is cvs diff -u one is cvs diff -u -b so we can see the diff ignore space
chagnes. 
I do need to change GetAppleEventTSMData which proposed by brade in her attachment
to static instead of class private since I don't want to introduce
nsMacEventHandler in that header file. and there are no technical reason it
should be a class memeber instead of a file static helper function.

brade, can you code review it again ?
(Assignee)

Comment 40

16 years ago
The latest patch don't run, it crash at
+    res = eventHandler->HandleUKeyEvent((PRUnichar*)unicodeText.get(),
text_size , event);


I need to find out what happen about it later.
brade, can you still review the formating stuff? (or not) 

(Assignee)

Comment 41

16 years ago
Created attachment 53109 [details] [diff] [review]
v5, fix the crash of getting the event handler from apple event.
(Assignee)

Comment 42

16 years ago
Comment on attachment 53036 [details] [diff] [review]
v4 of the diff of existing files

crash, see the v5 diff
Attachment #53036 - Attachment is obsolete: true
(Assignee)

Comment 43

16 years ago
Comment on attachment 53037 [details] [diff] [review]
v4 of the diff of existing files igore space changes (with -b in cvs diff)

crash , see the v5 diff for the real one
Attachment #53037 - Attachment is obsolete: true
(Assignee)

Comment 44

16 years ago
Comment on attachment 52965 [details]
code snippet of some factoring that might be done (with other cleanup as well)

integrate into v5 diff
Attachment #52965 - Attachment is obsolete: true
(Assignee)

Comment 45

16 years ago
brade, can you put your review comment here ?
I find out what happen to the v4 patch. The code you attached does not compiled, 
and I fix it in a wrong way. The v5 patch won't crash. Both keyboard and IME 
work perfect on MacOS X build. Once we pass the format review, I will spend more 
time on testing on MacOS 9 and get sr.
(Assignee)

Updated

16 years ago
No longer blocks: 77038
(Assignee)

Updated

16 years ago
Depends on: 77038

Comment 46

16 years ago
Created attachment 53285 [details] [diff] [review]
version 5.1 (whitespace cleanup and fix typos)

Comment 47

16 years ago
I attached a newer patch with some white space cleanup and fix some typos.  
Note:  I didn't change any lines that weren't already being touched for this bug.

There is one place in there where I added a comment with "ftang" in it.  I think
that issue should be addressed and then the comment removed.
(Assignee)

Comment 48

16 years ago
+      goto err2;  // FTANG:  SHOULD THIS REALLY BE:  goto err1;
Yes, it should

Comment 49

16 years ago
r=brade with version 5.1 and the fix noted in the previous comment
(as well as getting lots of testing)
(Assignee)

Updated

16 years ago
Attachment #53109 - Attachment is obsolete: true
(Assignee)

Comment 50

16 years ago
what we need now is
http://bugzilla.mozilla.org/attachment.cgi?id=52759&action=view
http://bugzilla.mozilla.org/attachment.cgi?id=52919&action=view
http://bugzilla.mozilla.org/attachment.cgi?id=53285&action=view (with one line
change from err2 to err1)

sfraser- can you sr= it ?
(Assignee)

Updated

16 years ago
Blocks: 104148
No longer blocks: 104056

Comment 51

16 years ago
ftang: it would be easier for me to review this if -
* You include everything in a single patch -- MacCVS Pro can show new files in
  a patch if you cvs add them first.
* Attachment 53285 [details] [diff] would be much easier to read if you diff ignoring whitespace.
(Assignee)

Comment 52

16 years ago
Created attachment 53741 [details] [diff] [review]
v3 of nsTSMStrategy.h + v4 of nsTSMStrategy.cpp + v5.1 of diff + (one line change err2 to err1)
(Assignee)

Comment 53

16 years ago
new patch is the same as the one reviewed by brade. I just combine them with
different cvs option. (Plus one fix from err2 to err1 as noted by brade)
(Assignee)

Comment 54

16 years ago
sfraser: please sr it 

Comment 55

16 years ago
Comment on attachment 53741 [details] [diff] [review]
v3 of nsTSMStrategy.h + v4 of nsTSMStrategy.cpp + v5.1 of diff + (one line change err2 to err1)

sr=sfraser
Attachment #53741 - Flags: superreview+
(Assignee)

Updated

16 years ago
Blocks: 104060
No longer blocks: 104148
(Assignee)

Comment 56

16 years ago
fixed and check in .
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
No longer blocks: 104060

Comment 57

16 years ago
This does appear to be working on Mac/2001112011 (0.9.6) on Mac OS 9. However,
the process is very awkward.

Suppose I select the Extended Roman (U) keyboard layout. If I switch to another
window and back, boom: the keyboard layout switches back to US.

This can't be how things are intended to work. If I switch to a Unicode keyboard
layout in Mozilla, it should stay that way, certainly for a given window, and
perhaps even for the application as a whole until changed.
(Assignee)

Comment 58

16 years ago
greg@tcp.com- file a seperate bug for that issue. It's seems a different bug. 

Updated

16 years ago
Blocks: 115887
Blocks: 284251
You need to log in before you can comment on or make changes to this bug.