Closed Bug 242315 Opened 21 years ago Closed 19 years ago

Patch to enable inline input in Japanese on BeOS

Categories

(Core :: Internationalization, defect)

x86
BeOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: koki, Assigned: sergei_d)

References

()

Details

(Keywords: fixed1.8.1, intl)

Attachments

(2 files, 10 obsolete files)

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) (deleted) — 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) (deleted) — 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
Attached patch Newest patch as Koki pointed to me (obsolete) (deleted) — Splinter Review
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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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
Attached patch One more try at this patch (deleted) — Splinter Review
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: 19 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.

Attachment

General

Created:
Updated:
Size: