Closed Bug 242315 Opened 20 years ago Closed 18 years ago

Patch to enable inline input in Japanese on BeOS

Categories

(Core :: Internationalization, defect)

Product:
Core
Component:
Internationalization
Platform:
x86
BeOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: koki, Assigned: sergei_d)

References

(
URL
)

Details

(Keywords: fixed1.8.1, intl)

Attachments

(2 files, 10 obsolete files)

English rough translation Rihatsu-san's solution
8.25 KB, text/plain
Details
One more try at this patch
13.49 KB, patch
sergei_d
: review+
asa
: approval1.8rc1+
darin.moz
: approval1.8.1+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.7b) Gecko/20040412 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.7b) Gecko/20040412 Firefox/0.8.0+

One of the main inconveniences of using Mozilla/Firefox in a Japanese
environment in BeOS is that it is not possible to enter text inline. This means
that when you switch to Japanese input, text is not entered directly into the
Mozilla window, but instead through a little pop-up window. Although this may
arguably be not a critical shortcoming, it is a quite annoying one.

The good news is that Rihatsu-san at JPBE.net has come up with a way to fix
this, and he has asked me to share it with the Bezilla development team in the
hope that his proposal is merged into the source tree of Mozilla.

Attachment ime.txt contains a rough translation of the solution that Rihatsu-san
posted at JPBE.net (http://jpbe.net/forum/viewtopic.php?p=5867#5867).

Attachment ime.zip has the sources (believe this is from the 1.6 trunk). The
added lines are identified by the comment "// Ryeha2" at the end of the line.

I hope you can understand and can make use of this. If not, feel free to ask and
I will convey your questions to Rihatsu-san. Also, if you need any testing of
binaries in BeOS, just let me know; I have a full team eager to test this
further and report problems.



Reproducible: Always
Steps to Reproduce:
N/A
Actual Results:  
Japanese input is done through an IME popup window.

Expected Results:  
With this patch, hopefully, it will be possible to enter Japanese directly into
the Mozilla window in BeOS.
English rough translation Rihatsu-san's solution posted at
http://jpbe.net/forum/viewtopic.php?p=5867#5867 (this is in Japanese).
Attachment ime.zip has the sources (believe this is from the 1.6 trunk). The
added lines are identified by the comment "// Ryeha2" at the end of the line.
jshin, could you take a look at this?  If not, who'd be the right person to?
Adding a few people who might be able to help.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: intl
Attached patch Diff file (obsolete) — — Splinter Review 
Rihatsu-san created this patch by loging in to
anonymous@cvs-mirror.mozilla.org:/cvsroot and using the following command:

cvs diff -uwp nsWindow.cpp nsWindow.h > nsWindow.cpp.patch

Thought this diff would be better than the full files previously posted for
this bug.

Koki
Thanks for the diff, but it would be even better to remove commented lines in
the patch and 'diff -u8 -p' would be better
As requested, posting an edited 'diff -u8 -p' patch with all comments removed.
Per the author (Rihatsu-san), all unnecessary printf have also been removed.
Comment on attachment 147628 [details] [diff] [review]
 'diff -u8 -p' patch with all comments & printf removed

> +void nsViewBeOS::DoIME(BMessage *msg)
...
> +uint32 *args = new uint32[argc];
that allocation could fail
> +msg->Flatten((char*)args, size);
then this would crash
or at least, that's the theory

> +BezillaIME::RunIME(uint32 *args, nsWindow *target, BView *view)
...
> +mText  = new PRUnichar[mCount+1];
  ^^ not checked
> +cutf16(src, &srcLen, mText, &dstLen);
would crash if mText allocation failed

while utf8_str_size / utf8_str_len / cutf16 look ok to me, i want someone else
to concur (smontagu, jshin, ...). Also, the name |cutf16| is a bit strange,
something more verbose might be better. as an English speaker, i first parsed
it as |cut,f,16| and got confused.
(In reply to comment #8)

> while utf8_str_size / utf8_str_len / cutf16 look ok to me, i want someone else
> to concur (smontagu, jshin, ...). 

Before looking in detail I would like to understand why you need to roll your
own routines instead of using the ones in xpcom/string/
momoi-san, can you help with this? If you could contact Rihatsu-san directly
about the UTF-8 conversion, it would be great.
This is an attempt to convey Rihatsu-san's responses (at
http://jpbe.net/forum/viewtopic.php?p=6149#6149).

*********
> Also, the name |cutf16| is a bit strange...
I am a bit at a loss as to what would be acceptable/appropriate for an English
speaker. Perhaps someone could recommend a good name here?

> why you need to roll your own routines instead of using the ones in xpcom/string/
I simply did not know how to use xpcom/string.

In the setion cutf16, change to mText = ToNewUnicode(NS_ConvertUTF8toUCS2(src));
delete[] mText can be changed to nsMemory::Free(mText);

Not sure what I will do with utf8_str_len and utf8_str_size yet, so this is
on-hold (until I figure it out?).

***********

I hope this rough "translation" makes some sense...

It definitely does.  I'd actually recommend using an nsString for mText.  Then
the code can become:

  CopyUTF8toUTF16(src, mText);

where mText is currently allocated, mText.get() for places where we need a
PRUnichar*, mText.Truncate() when we want to clear the string, et.

Alternately, you could keep mText as-is and instead of

  mText = ToNewUnicode(NS_ConvertUTF8toUCS2(src));

use

  mText = UTF8ToNewUnicode(src);

(In reply to comment #11)

> Not sure what I will do with utf8_str_len and utf8_str_size yet, so this is
> on-hold (until I figure it out?).

For utf8_str_len I think you can #include "nsUTF8Utils.h"and use
CalculateUTF8Length, like at
http://lxr.mozilla.org/seamonkey/source/xpcom/string/src/nsReadableUtils.cpp#230
(Boris or Jungshik, please correct me if I am wrong).
utf8_str_size is anyway only used internally by cutf16 and utf8_str_len, right?
(In reply to comment #13)
> For utf8_str_len I think you can #include "nsUTF8Utils.h"and use
> CalculateUTF8Length, like at

Reply from Rihatsu-san (at http://www.jpbe.net/forum/viewtopic.php?p=6170#6170).

*********
I was able to verify that CalculateUTF8Length works, so I plan to use it.
However, I always get a "always_inline' attribute directive ignored" warning: is
this attribute required?

Also, in the code for [CalculateUTF8Length] the following appears:

else if ( UTF8traits::is4byte(*p) ) {
p += 4;
++mLength;
}

I cannot understand this, so I am a bit reluctant (to use it?).
********

in mozilla/widget/gfx/beos/nsRenderingContextBeOS.cpp we use macros and inlines
for utf-8 processing for speed purpose. Borrowed from old BeNewsLetters.
You can look there, though, i don't think that code in widget has some speed
requirements, so probably better is to use regular mozilla tools
Sorry, wrong path in previous mesage. Must be:
mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp
(In reply to comment #14)

> Also, in the code for [CalculateUTF8Length] the following appears:
> 
> else if ( UTF8traits::is4byte(*p) ) {
> p += 4;
> ++mLength;
> }
> 
> I cannot understand this, so I am a bit reluctant (to use it?).

It's very bad that this isn't documented properly in the file. The "length of a
UTF-8 string" calculated by CalculateUTF8Length is not the number of codepoints
in the string but the length of the UTF-16 string with the same codepoints.
Because a UTF-8 sequence of 4 bytes represents a codepoint greater than 0xFFFF,
it will become a surrogate pair in the UTF-16 string, hence |++mLength|.

This doesn't happen with is5byte and is6byte because these are illegal UTF-8
sequences (greater than 0x10FFFF) so get converted to a single replacement
character.

I hope this is clear.
I have some questions to Rihatsu-san.
1)What for is B_NAVIGABLE flag in our case?
2)Why not to set both fags B_NAVIGABLE|B_INPUT_METHOD_AWARE at nsViewBeOS
creation but in MakeFocus? Does setting those falgs at creation affects badly
some other aspects?
3)What happens if you just set _B_INPUT_METHOD_AWARE flag, but don't add any
code, and try to type japanese characters? Some screenshots, pls, is there is
any efect.
4)BeNewsletter says that with B_INPUT_METHOD_AWARE flag events receive BView on
one of two ways - via B_INPUT_EVENT message and via BView:KeyDown(). What happens 
 in KeyDown() in your code? Does it receive anything too? Or getting
B_INPUT_EVENT message in MessageReceives prevents call of that KeyDown() hook?

5)Did you estimate possibility to implement IME in way like it is done with that
little proxy window but inside BeOS widget code? I mean - process whole message
inside nsViewBeOS method and call there BView::KeyDown/KeyUp functions (or post
BMessages  which will result in calling KeyUp/Down()).

P.S. That's very sad Rihatsu-san don't speak nor English neither Russian. Ehheh,

P.P.S Do you know any article about BeOS input method usage besides only article
i found in Volume III, Issue 7, February 17, 1999 of Be Newsletter?
(In reply to comment #18)
> I have some questions to Rihatsu-san.

I translated the questions into Japanese and posted it on the JPBE.net forum for
Rihatsu-san to see and hopefully answer.

http://www.jpbe.net/forum/viewtopic.php?p=6861#6861

If he replies, I will post his answers in English here.

Koki
(In reply to comment #19)
> (In reply to comment #18)
> > I have some questions to Rihatsu-san.
> I translated the questions into Japanese and posted it on the JPBE.net forum 
for
> Rihatsu-san to see and hopefully answer.
> http://www.jpbe.net/forum/viewtopic.php?p=6861#6861
> If he replies, I will post his answers in English here.
> Koki

Here is a translation of Rihatsu-san's reply. I am afraid that this 
translation may not be very acurrate, as honestly the content is very 
technical for me. I am posting it nevertheles in the hope that it could be of 
some use. Here I go.

*******
1) I added it since, according to the Developer's Guide Vol. 1, this is a flag 
that indicates that BView can be the focus view for a keyboard operation, and 
therefore a view for key input was needed. It is possible to enter text 
without this flag, so maybe it is not required.

2) This is to have the least effect on the original (code?). With this, 
MakeFocus is a false view as in the original (code?). If added during 
creation, everything will make this flag constantly in an ON status. Setting 
the flag during creation does not affect other aspects. (Note: these last two 
sentences sound contradicting, so I am not sure if they make sense.)

3) Nothing would happen. So, I cannot send you a screenshot of "nothing 
happening". :-)

Raising the B_INPUT_METHOD_AWARE is just to indicate that the following 
messages will be processed. If you just indicate but do not do anything, 
nothing would happen. In that case, the BMessage would be discarded.

4) It is not in one of to ways, but actually from both ways. It comes from 
both B_INPUT_EVENT as well as from BView:KeyDown(), so it is actually telling 
you to process both without failure. Of course, the content is different. What 
this means is that the receiving side cannot decide whether to process only 
B_INPUT_EVENT or to have everything sent via BView:KeyDown(). Which way it 
comes is decided by the module (InputMethod) originating from 
BInputServerMethod. 
 
5) (Note: I may not have translated your question properly, so it would not be 
surprising if his answer is a bit off). The inline implementation is in the 
View side. In other words, all messages are processed in nsViewBeOS, and 
BView::KeyDown/KeyUP functions are called (or  BMessages are used to call 
KeyUp/KeyDown()).

I am not sure what you are trying to ask, but I will try to answer anyway.

IME is a mechanism to implement inline (input). This displays the text that 
has been entered or converted, but it is still not considered to be an input.

For example, let's say that the letter A is received via B_INPUT_EVENT. This 
is still not an input, so it cannot be processed using BView::KeyDown/KeyUp. 
But it still has to be displayed. If you do not display the letter, the person 
typing will (mistakenly) think that the letter that he/she typed was not 
received. So, can we display the letter A in insert mode in the current cursor 
position from nsViewBeOS? I am sure you understand this better than me, but to 
the extent of my knowledge, the operation and information for displaying is in 
nsWindow.cpp. You may be able to take that information and operate it in 
nsViewBeOS, I do not think there is any advantage in doing so. WIN32, OS2 and 
GTK they all process IME in nsWindow. I cannot think of any reason that would 
merit processing INE in nsViewBeOS.

The converted entry can be processed via BView::KeyDown/KeyUp. However, having 
BView::KeyDown/KeyUp recieve again what has already been received does not 
make much sense to me.
***********

Adding Nakano-san to Cc who may or may not have BeOS around. Even if he doesn't,
 he may be of help in moving this forward.
Sorry, my system and my knowledge of IME programming are Windows only.
Attached patch Updated patch (obsolete) — — Splinter Review 
This is an updated patch. It should apply cleanly to AVIARY, and it will do
that for HEAD as well. I have made one change though: nsCompositionEvent
doesn't have a 'compositionMessage'  (anymore). Will it work this way as well?

Niels
Blocks: 266252
Newest patch which will be included in build 1.0.0-d3 .

BTW, could someone either make me QA contact or assign this bug to me, or
alternatively simply obsolete the previous patches.

Thanks.
Attachment #163538 - Attachment is obsolete: true
Regarding comment 18 , Sergei are all your concerns addressed at this point?
Attached patch Properly whitespaced patch (obsolete) — — Splinter Review 
Sorry to bug you (yet again). This time I attached a properly whitespaced patch
(in my opinion). The only thing I like to see changed is the name of the class
(BeZillaIME), which should become nsIMEBeOS, IMO. Your opinions? I'll be doing
a build tomorrow with this patch included for testing.
Attachment #163748 - Attachment is obsolete: true
Niels, in this file we use (at least try to use consistently) GNU style for
btackets.
foo1
{
 foo2
 {
 }
}

Agree about names. "Bezilla" looks like argo.

Okay, I am changing all the BezillaIME into nsIMEBeOS (which isn't particularly
esthetically pleasing, but it should do). 

Another thing I'm not sure about is the global variable be_IME. Shouldn't this
become a static member accessible by a static function of the nsIMEBeOS class?

For the more knowledgeble among us, is the patch code-wise correct?
(In reply to comment #28)
> Okay, I am changing all the BezillaIME into nsIMEBeOS (which isn't particularly
> esthetically pleasing, but it should do). 
> 
> Another thing I'm not sure about is the global variable be_IME. Shouldn't this
> become a static member accessible by a static function of the nsIMEBeOS class?
> 
> For the more knowledgeble among us, is the patch code-wise correct?

Here is a message from Rihatsu-san...

********
I have no objections about changing BeZillaIME to nsIMEBeOS.
I chose to use the BeZillaIME class as a way to reduce as much as possible the
number of changes in nsWindow. In reality, it would be better to make it a
member of nsWindow like in the other platforms. Plus, since BeZillaIME is a
class, it means that it duplicates functions from nsWindows, which is a waste.

BeZillaIME is a global class. It is the only one in Mozilla. If it were to be
made a member of nsWindow, then it would exist for every nsWindow. The way that
BeOS handles input methods currently does not require this; however, looking at
how the mozilla devs are testing other new input methods in Windows, it may be
wise to keep the class on a session basis.
*******

I hope that my translation makes sense. Feel free to ask if it does not. :-)
(In reply to comment #28)
> Another thing I'm not sure about is the global variable be_IME. Shouldn't this
> become a static member accessible by a static function of the nsIMEBeOS class?

More more reply from Rihatsu-san...

******
If you make it global, the relationaship between nsWindow, be_IME and Input
Method becomes n:1:1. If you make it a static member, then the relationship
becomes n:n:1.

The problem is that each nsIMEBeOS holds the information related to Input
Method, but Input Method does not. Which means that if you change focus in the
middle of a session, then you may have a problem.

So, I think it is necessary to synchronize between the nsIMEBeOS of the nsWindow
that looses focus and the nsIMEBeOS of the nsWindow that gains focus.
********

Well, I don't actually mean a functional change. I rather mean a stylistic
change. Let's give the nsIMEBeOS class the following members:

public:
    static nsIMEBeOS *GetIME() 
    {
        if(mIME == 0)
        {
            mIME = new nsIMEBeOS();
        }
        return mIME;
    } 
private:
    static nsIMEBeOS *mIME = 0;


This would opt rather for a stylistical change as the be_IME global could be
deleted. It would in no way mean that the workings of the patch should be changed.

I just wonder what is stylistically preferable.
(In reply to comment #31)
> Well, I don't actually mean a functional change. I rather mean a stylistic
> change. Let's give the nsIMEBeOS class the following members:
> 
> This would opt rather for a stylistical change as the be_IME global could be
> deleted. It would in no way mean that the workings of the patch should be changed.
> 
> I just wonder what is stylistically preferable.

Comment from Rihatsu-san...

********
I now understand that you meant to make it a static member of nsWindow. 
Yes, this is better than making it global.
********

Comment from Koki: it may be that my translation to Rihatsu-san was not clear
enough, in which case I apologize. I hope this moves us one step closer to
committing this patch. :-)

Koki

(In reply to comment #32)
> > I just wonder what is stylistically preferable.
> 
> Comment from Rihatsu-san...
> 
> ********
> I now understand that you meant to make it a static member of nsWindow. 
> Yes, this is better than making it global.
> ********

About the functional part: I think it works perfectly like this. If we only need
one communication object, why bother moving that into BWindow? Of course I don't
 have anything to say about this, but we'll feel the general consensus once it
is on for review.

> Comment from Koki: it may be that my translation to Rihatsu-san was not clear
> enough, in which case I apologize. I hope this moves us one step closer to
> committing this patch. :-)

Don't worry about it. I will have to see whether I have some time to fix it
today, otherwise it will be tomorrow morning. I'll put out a build as soon as
possible then. 
setting Neils as owner.
Assignee: smontagu → Niels.Reedijk
Status: NEW → ASSIGNED
Attached patch Input patch without the global (obsolete) — — Splinter Review 
Here's the input patch that is included in the development release of Firefox
1.0.0 - d3. If this one works, it will also be flagged for review...
Attachment #163757 - Attachment is obsolete: true
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global

Please review this bug. It appears to work.
Attachment #164232 - Flags: review?(sergei_d)
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global

Please review this bug. It appears to work.
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global

Starting polishing.
1)BTNCLCK: case in CallMethod - access to arguments must be performed AFTER
test for number of arguments

2)nsViewBeOS::MakeFocus()
Inspite reports say that patch is working, it is unclear for me, is it ok to
rely on "focused" argument BEFORE we set actual focus state

3)Question rather about style - for all other classes we've put decalration in
*.h file, so IME here appears to be exception - both decalration and
implementation are in *.cpp.
Is there any serious reason behind it?
Attachment #164232 - Flags: review-
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global

see previous comment
Attachment #164232 - Flags: review?(sergei_d)
(In reply to comment #38)
> (From update of attachment 164232 [details] [diff] [review])
> Starting polishing.
> 1)BTNCLCK: case in CallMethod - access to arguments must be performed AFTER
> test for number of arguments

Hmmm, definately true, I will change that.

> 2)nsViewBeOS::MakeFocus()
> Inspite reports say that patch is working, it is unclear for me, is it ok to
> rely on "focused" argument BEFORE we set actual focus state

I'll look to that. 

> 3)Question rather about style - for all other classes we've put decalration in
> *.h file, so IME here appears to be exception - both decalration and
> implementation are in *.cpp.
> Is there any serious reason behind it?

I think the main reason is that the class is only used in that cpp file. It
would be of no use to actually put it in a header. At the other hand, other
classes used in one file are also in headers, so it would be a question of
policy. As in this case, I don't think it would be needed to put the class
definition in a header. It's fine this way, and it doesn't polute headers.

I'll give it a swing when I have more time.

Niels
3)class declaration inside *.cpp
If there isn't any special idea behind it, i think we should follow MS Win and
GTK example in that aspect. Look pls there.

Also, bit off-topic -looking at your patch i noticed my own old mistake in
CallMethod() code. There is ONWORKSPACE: case in switch, but it is wrong - must
be something like nsWindow::ONWORKSPACE  there. Can you file new bug on that?
I will quickly review it, and if tree is open, will try for check patch in
before i leave my home (will be in Sweden from 7-th to 21-th October). So new
IME patch will be maid against new code with that stupid bug fixed.
(In reply to comment #41)
> 3)class declaration inside *.cpp
> If there isn't any special idea behind it, i think we should follow MS Win and
> GTK example in that aspect. Look pls there.

The windows port has private classes as well. Check
/mozilla/widget/src/windows/nsWindow.cpp line 549 (nsAttentionTimerMonitor). So
we are following conventions here.
(In reply to comment #40)
> > 2)nsViewBeOS::MakeFocus()
> > Inspite reports say that patch is working, it is unclear for me, is it ok to
> > rely on "focused" argument BEFORE we set actual focus state

I don't think I really understand the problem here. You mean that the:

	if (focused)
		SetFlags(Flags()|B_NAVIGABLE|B_INPUT_METHOD_AWARE);
	else
		SetFlags(Flags() & ~(B_NAVIGABLE|B_INPUT_METHOD_AWARE));

should be after:
	if (!IsFocus() && focused)
		BView::MakeFocus(focused); ?
>should be after:
I looked at code again.
Actually it may be where it is now, i confused it with nsWindow::SetFocus()
method, where we may reject request to set focus.

Though, i have now questions to Rihatsu-san again (sorry for my slowness:).

Fact:B_NAVIGABLE flag allows to set focus to given view by TAB key. 

Problem #1: We set that flag when we already got focus, and unset when focus is
lost. Is there any sense in that?

Problem #2:  We haven't that flag set in BView constructor, but switching focus
by TAB works anyway - Mozilla handles it by self. Do we really need it?

Problem #3:
 	case nsWindow::BTNCLICK :
+		if (((int32 *)info->args)[3] == 1) 
+			nsIMEBeOS::GetIME()->DispatchCancelIME();

Is it really proper trigger-event for DispatchCancelIME ? 
As BTNCLCK with clicks==1 may result in very various events - getting focus,
loosing focus, moving text caret, nothing.

Koki, are you watching this thread?
Attached patch Patch for 1.0.0-d4 (obsolete) — — Splinter Review 

        Attachment #164232 -
        Attachment is obsolete: true

    
  

        Attachment #164930 -
        Attachment description: Rejected patch, but updated to latest cvs → Patch for 1.0.0-d4

    
    

    
  
(In reply to comment #44)
> Koki, are you watching this thread?

Sorry fyysik. I dropped the ball on this one. Rihatsu-san had replied this long
back... Here is a translation of his reply. I hope this is useful.

*********
> Fact:B_NAVIGABLE flag allows to set focus to given view by TAB key. 

B_NAVIGABLE also indicates that BView can become a focus view for a keyboard
operation. In other words, if there is no B_NAVIGABLE, it means that it is not
possible to obtain focus view from a keyboard operation. That is how I am using
it. For the program, moving the focus with TAB is not important.

> Problem #1: We set that flag when we already got focus, and unset when focus is
> lost. Is there any sense in that?

It may not have any meaning, but it does not create a problem either. I do not
understand, however, why you ask this question. Is it about the SetFlags()
portion of MakeFocus()? Expressions such as "got focus" and "unset when focus is
lost" impply things happening in past tense, but note that when entering
MakeFocus() the focus itself has not changed yet. However, this may depend on
whether by "focus" you refer to the focus on Mozilla or that on BeOS.

> Problem #2:  We haven't that flag set in BView constructor, but switching focus
> by TAB works anyway - Mozilla handles it by self. Do we really need it?

I guess you are referring to B_NAVIGABLE. As mentioned in my reply to Problem
#1, if there is no B_NAVIGABLE, it means that it is not possible to obtain focus
view from a keyboard operation. Getting rid of this in Mozilla may not pose any
problems.

Problem #3:
 	case nsWindow::BTNCLICK :
+		if (((int32 *)info->args)[3] == 1) 
+			nsIMEBeOS::GetIME()->DispatchCancelIME();

Is it really proper trigger-event for DispatchCancelIME ? 
As BTNCLCK with clicks==1 may result in very various events - getting focus,
loosing focus, moving text caret, nothing.

I do not understand the purpose of your question? Are you suggesting that more
code be added to handle other (possible) events from BTNCLCK? nsIMEBeOS needs to
catch any focus moves. When you click, it is expected that the focus will move.
The purpose here is to catch the loss of focus. The number "1" has no particular
meaning. That is because a left click is the one used to move the focus. As long
as it is possible to catch the loss of focus, it is OK to have it in another
part (of the code).
*************

    
  

        Attachment #147596 -
        Attachment is obsolete: true

    
  

        Attachment #147480 -
        Attachment is obsolete: true

    
  

        Attachment #147628 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        updated patch for 1.6.1a (obsolete)
        — 
          — Splinter Review
      

    


  
changed code to reflect changes in nsGUIEvent (bug 289940 and 296036).	Needs
eye of seasoned dev to see if patch correctly reflects nsGUIEvent updates.

    
    

    
  
Comment on attachment 164930 [details] [diff] [review]
Patch for 1.0.0-d4

Newer patch required.

        Attachment #164930 -
        Attachment is obsolete: true

    
    

    
  
neilx,   With all the work touching nsWindow, it seems easier to let the
work-in-progress settle first, then update this patch.  At this point, I believe
only tqh's nsWindow cleanup under bug 296856 is still actively in process.  If
he's almost done, I'll wait until that's committed.  If you want something
sooner, I can update but make this dependent on 296856.  Let me know which
method is better for the community.

    
  
No longer blocks: 266252

    
  
Blocks: 311032

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch against current tree (obsolete)
        — 
          — Splinter Review
      

    


  
bit refactored (class declaration moved in *.h) and ifdefed patch.
Far from final, more refactoring coming, mostly due code in BTNCLICK and
MakeFocus. Maybe something more.
Put here for reference and for tigerdog and nielx convinience.
Assignee: Niels.Reedijk → sergei_d

        Attachment #196286 -
        Attachment is obsolete: true

    
    

    
  


  

        Attachment #200223 -
        Flags: review?(sergei_d)
Summary: Patch to enable inline input in Japanese → Patch to enable inline input in Japanese on BeOS

    
    

    
  
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch

r=sergei_d

        Attachment #200223 -
        Flags: review?(sergei_d) → review+

    
    

    
  
Koki-san:

The patch doesn't have nsTextFrame.cpp. I think that we need it.

    
    

    
  
landed in trunk
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.104; previous revision: 1.103
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.41; previous revision: 1.40
done

Will close after 1.8* checkin

    
    

    
  
You should close this bug by fixed on trunk.
And if fixed on 1.8 branch, you should use fixed1.8 and verified1.8 keywords.

See http://developer.mozilla.org/devnews/index.php/2005/08/18/branch-keywords/

    
    

    
  
Masayuki-san:
nsTextFrame.cpp addition should be separate bug and patch IMHO.
I will be glad if you point me at proper component in CORE to submit it, and
maybe i will ask you for review, if you are allowed to do reviews on this topic.

Truth is it's very easy for us at moment to do patches in BeOS-only folders, but
quite hard (due formal reasons) in such components as layout/toolkit/gkview etc

    
    

    
  
Comment on attachment 199759 [details] [diff] [review]
patch against current tree

obsoleting

        Attachment #199759 -
        Attachment is obsolete: true

    
    

    
  
closing bug
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

    
    

    
  
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch

asking approval for 1.8 branch

        Attachment #200223 -
        Flags: approval1.8rc1?

    
    

    
  
If this patch goes to 1.8 branch, we need to change nsTextFrame.cpp on 1.8
branch too. Because we may not get feedback for composition string. This is
serious regression.

    
    

    
  
AT this point I would advice against applying this patch in the 1.8 branch. I'm
still working out the kinks caused by the DnD patch, I can't work on this one as
well. 

    
  

        Attachment #200223 -
        Flags: approval1.8rc1? → approval1.8rc1+

    
    

    
  
I filed bug 313174 for rendering of composition string.

    
    

    
  
They way you use GetMouse doesn't work properly, it's to bad you sneak things in
instead of discussing them. Even if GetMouse remove the last message it may be a
MouseUp event. This makes me disappointed.

    
    

    
  
tqh, i think your notice is about another bug, isn' it?

    
    

    
  
Sorry, tqh, now I understand your disppointment.
Probably Koki got (i'm really guilty) diff against my version of nsWindow, not CVS.
So unreviewed (and without plans to review that) patch from Bug 312755 landed
here, which wasn't my intention at all.

    
    

    
  
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch

Requesting approval for the MOZILLA_1_8_BRANCH

This is a BeOS-only change and will not affect any other platform.

        Attachment #200223 -
        Flags: approval1.8.1?

    
    

    
  
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch

a=darin on behalf of drivers

        Attachment #200223 -
        Flags: approval1.8.1? → approval1.8.1+

    
    

    
  
Sergei, could you commit this to the branch after bug 312660?

(It's this one instead of bug 314687)

    
    

    
  
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.91.4.14; previous revision: 1.91.4.13
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.35.4.9; previous revision: 1.35.4.8
done 
Keywords: fixed1.8.1

          You need to log in
          before you can comment on or make changes to this bug.