Closed
Bug 242315
Opened 21 years ago
Closed 19 years ago
Patch to enable inline input in Japanese on BeOS
Categories
(Core :: Internationalization, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: koki, Assigned: sergei_d)
References
()
Details
(Keywords: fixed1.8.1, intl)
Attachments
(2 files, 10 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
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.
Reporter | ||
Comment 1•21 years ago
|
||
English rough translation Rihatsu-san's solution posted at
http://jpbe.net/forum/viewtopic.php?p=5867#5867 (this is in Japanese).
Reporter | ||
Comment 2•21 years ago
|
||
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.
Comment 3•21 years ago
|
||
jshin, could you take a look at this? If not, who'd be the right person to?
Comment 4•21 years ago
|
||
Adding a few people who might be able to help.
Reporter | ||
Comment 5•20 years ago
|
||
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
Comment 6•20 years ago
|
||
Thanks for the diff, but it would be even better to remove commented lines in
the patch and 'diff -u8 -p' would be better
Reporter | ||
Comment 7•20 years ago
|
||
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.
Comment 9•20 years ago
|
||
(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/
Comment 10•20 years ago
|
||
momoi-san, can you help with this? If you could contact Rihatsu-san directly
about the UTF-8 conversion, it would be great.
Reporter | ||
Comment 11•20 years ago
|
||
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...
Comment 12•20 years ago
|
||
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);
Comment 13•20 years ago
|
||
(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?
Reporter | ||
Comment 14•20 years ago
|
||
(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?).
********
Assignee | ||
Comment 15•20 years ago
|
||
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
Assignee | ||
Comment 16•20 years ago
|
||
Sorry, wrong path in previous mesage. Must be:
mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp
Comment 17•20 years ago
|
||
(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.
Assignee | ||
Comment 18•20 years ago
|
||
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?
Reporter | ||
Comment 19•20 years ago
|
||
(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
Reporter | ||
Comment 20•20 years ago
|
||
(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.
***********
Comment 21•20 years ago
|
||
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.
Comment 22•20 years ago
|
||
Sorry, my system and my knowledge of IME programming are Windows only.
Comment 23•20 years ago
|
||
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
Comment 24•20 years ago
|
||
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.
Updated•20 years ago
|
Attachment #163538 -
Attachment is obsolete: true
Comment 25•20 years ago
|
||
Regarding comment 18 , Sergei are all your concerns addressed at this point?
Comment 26•20 years ago
|
||
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.
Updated•20 years ago
|
Attachment #163748 -
Attachment is obsolete: true
Assignee | ||
Comment 27•20 years ago
|
||
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.
Comment 28•20 years ago
|
||
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?
Reporter | ||
Comment 29•20 years ago
|
||
(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. :-)
Reporter | ||
Comment 30•20 years ago
|
||
(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.
********
Comment 31•20 years ago
|
||
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.
Reporter | ||
Comment 32•20 years ago
|
||
(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
Comment 33•20 years ago
|
||
(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.
Updated•20 years ago
|
Status: NEW → ASSIGNED
Comment 35•20 years ago
|
||
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 36•20 years ago
|
||
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 37•20 years ago
|
||
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global
Please review this bug. It appears to work.
Assignee | ||
Comment 38•20 years ago
|
||
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-
Assignee | ||
Comment 39•20 years ago
|
||
Comment on attachment 164232 [details] [diff] [review]
Input patch without the global
see previous comment
Attachment #164232 -
Flags: review?(sergei_d)
Comment 40•20 years ago
|
||
(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
Assignee | ||
Comment 41•20 years ago
|
||
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.
Comment 42•20 years ago
|
||
(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.
Comment 43•20 years ago
|
||
(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); ?
Assignee | ||
Comment 44•20 years ago
|
||
>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?
Comment 45•20 years ago
|
||
Attachment #164232 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #164930 -
Attachment description: Rejected patch, but updated to latest cvs → Patch for 1.0.0-d4
Reporter | ||
Comment 46•20 years ago
|
||
(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).
*************
Updated•20 years ago
|
Attachment #147596 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #147480 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #147628 -
Attachment is obsolete: true
Comment 47•19 years ago
|
||
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 48•19 years ago
|
||
Comment on attachment 164930 [details] [diff] [review]
Patch for 1.0.0-d4
Newer patch required.
Attachment #164930 -
Attachment is obsolete: true
Comment 49•19 years ago
|
||
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.
Assignee | ||
Comment 50•19 years ago
|
||
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
Reporter | ||
Comment 51•19 years ago
|
||
Attachment #200223 -
Flags: review?(sergei_d)
Updated•19 years ago
|
Summary: Patch to enable inline input in Japanese → Patch to enable inline input in Japanese on BeOS
Assignee | ||
Comment 52•19 years ago
|
||
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch
r=sergei_d
Attachment #200223 -
Flags: review?(sergei_d) → review+
Comment 53•19 years ago
|
||
Koki-san:
The patch doesn't have nsTextFrame.cpp. I think that we need it.
Assignee | ||
Comment 54•19 years ago
|
||
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
Comment 55•19 years ago
|
||
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/
Assignee | ||
Comment 56•19 years ago
|
||
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
Assignee | ||
Comment 57•19 years ago
|
||
Comment on attachment 199759 [details] [diff] [review]
patch against current tree
obsoleting
Attachment #199759 -
Attachment is obsolete: true
Assignee | ||
Comment 58•19 years ago
|
||
closing bug
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 59•19 years ago
|
||
Comment on attachment 200223 [details] [diff] [review]
One more try at this patch
asking approval for 1.8 branch
Attachment #200223 -
Flags: approval1.8rc1?
Comment 60•19 years ago
|
||
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.
Comment 61•19 years ago
|
||
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.
Updated•19 years ago
|
Attachment #200223 -
Flags: approval1.8rc1? → approval1.8rc1+
Comment 62•19 years ago
|
||
I filed bug 313174 for rendering of composition string.
Comment 63•19 years ago
|
||
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.
Assignee | ||
Comment 64•19 years ago
|
||
tqh, i think your notice is about another bug, isn' it?
Assignee | ||
Comment 65•19 years ago
|
||
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 66•18 years ago
|
||
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 67•18 years ago
|
||
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+
Comment 68•18 years ago
|
||
Sergei, could you commit this to the branch after bug 312660?
(It's this one instead of bug 314687)
Assignee | ||
Comment 69•18 years ago
|
||
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.
Description
•