GNOME Bugzilla – Bug 348638
[calendar] unable to remove pre-edit buffer cleanly in day view
Last modified: 2013-09-13 00:52:03 UTC
Please describe the problem: When a mistake is made during input, it is not possible to remove the characters from the pre-edit buffer in the day view of calendar. Steps to reproduce: 1.in any of the CJKI locale, start evolution-> calendar (eg ja_JP.UTF-8) 2.use the default day view 3.select 2PM, enter a 4.activate SCIM using Ctrl-SPACE 5.enter 'kaizoku' follow by backspace to remove everything entered earlier Actual results: unable to remove pre-edit buffer Expected results: able to remove pre-edit buffer cleanly Does this happen every time? yes Other information:
Upstream bug https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=178295 Added keyword - i18n
Created attachment 69579 [details] [review] Patch for the bug Patch that solves the problem
Ikezoe san, can you please review this patch? Though i'll upload a new one which will match the coding conventions, kindly just review it for correctness & if possible, give it a test. Thanks, makuchaku
Thank you, I just take a look.
Thanks :)
Uhm, this problem is ticklish... IMHO, application should not consider the behaviour whether preedit string is commited or cleared when focus out. One or two years ago, I received the patch that preedit string is *commited* when focus out for scim-anthy from SUSE developer. He think that preedit string should be committed when focus out. For the reason, I think that application entrust the behaviour for preedit string to input method. As far as I know, scim-anthy has maybe a such option.
Oops! I'am sorry, I completely misunderstood this bug.. Please forget my previous comment... sigh.
Oh, no issues :)
makuchaku, Now I can understand this problem. The problem is that preedit string is not drawn if the preedit length is 0. So insert_preedit_text () draws 0 length preedit string if the preedit length is 0.
the following change fix the problem. @@ -294,7 +294,7 @@ insert_preedit_text (EText *text) g_string_prepend_len (tmp_string, text->text,length); - if (text->preedit_len) + /* if (text->preedit_len) */ gtk_im_context_get_preedit_string (text->im_context, &preedit_string, &preedit_attrs, NULL); @@ -306,7 +306,8 @@ insert_preedit_text (EText *text) cpos = g_utf8_offset_to_pointer (text->text, text->selection_start) - text->text; - if (preedit_length) { + /* if (preedit_length) { */ + { g_string_insert (tmp_string, cpos, preedit_string); reset_layout_attrs (text);
Hi, is my patch not working?
Can you please attach it as a patch? Thanks :)
(In reply to comment #11) > Hi, is my patch not working? I am sorry, I haven't tested your patch, just read it. Because your approach is trick. I do not understand why 0 length preedit sring (i.e. "") is handled specialy. I think it is better to handle 0 length preedit string as usual. Maybe you misunderstand that gtk_im_context_get_preedit_srting returns NULL string, original code has also the same misunderstanding.
Created attachment 69791 [details] [review] patch for makuchaku
Created attachment 69799 [details] [review] clean patch
What was happening in previous code was that even when the preedit_len variable was zero, insert_preedit_text was being called & was doing un-necessary string operations, including insertion of preedit. So i just made sure that insert_preedit_text only gets called when preedit len is not zero. & then reset_layout is used to clear any leftovers, which was the actaul bug.
Hi, I applied your patch with a clean e-text.c fetched from cvs head. It does solves this bug, but again creates the problem of preedit replication, which my patch does not creates. Please test my patch. Thanks, Mayank
Created attachment 69804 [details] problem after applying hiroyuki's patch
Hiroyuki, this preedit replication is in a very strange way (as depicted in screenshot). All the text widgets which had been edited, receive a copy of preedit. If you apply my patch after applying patches for bug #264485, this bugs gets resolved perfectly. Please check, Thanks.
That's natural. Your patch includes the same fix as my patch in bug 264485.
(In reply to comment #16) I don't think your patch is wrong. It does not fit to API (gtk_im_context_get_preedit_string) because the function does not handle the condition of none preedit string specially. If gtk_im_context_get_preedit_string returns NULL value when there is no preedit string, your patch is reasonable for me. Hmm, the name of insert_preedit_string is not good. update_preedit_string is better. This name is reasonable for you?
I was wrong. 0 length preedit string is treat as individual, considering performance. I rewrote my patch, attach it.
Created attachment 69903 [details] [review] revised patch
Hi, I tested your patch with one of working patches of bug 264485 & the same result as http://bugzilla.gnome.org/show_bug.cgi?id=348638#c18 is observerd. Follow these steps to reproduce the problem 1) type "a" in 4 text widgets to make them editable 2) go to first text widget, activate scim-anthy & type "aka" 3) start deleting the preedit buffer with backspace till the last of "aka" is deleted 4) again type "aka" in the same widget & let it remain in preedit mode 5) select any other widget & type again "aka" & leave it in preedit mode 6) click in any of the rest 3 text widgets & see the string "aka" getting replicated. NOTE - make sure your preedit is not cleared by a popup or a focus out or you wont be able to reproduce this. Thanks, Makuchaku
makuchaku, the problem you mentioned is about bug 264485, not this bug?
Yes, but this patch is adding to that bug... Okay, lets do it once again. Can you please tell which patch from bug 264485 should I apply before applying this patch? Also, if I apply my patch from bug 264485 & then apply my patch from this bug, both problems get resolved :) Thanks, Makuchaku
(In reply to comment #26) > Yes, but this patch is adding to that bug... "that bug" is bug 264485? My patch in this bug is just for *this* bug. The patch does not unsurprisingly fix bug 264485.
I see your point. Okay, Ikezoe san, since both the patches work (I just tested them with scim-anthy & uim anthy), you being a more experienced developer, you can chose the better of them & then we can move the Evo team to include them to upstream. Thanks for your time, makuchaku
Hi Ikezoe san, Can you please update bug status... which patch have you chosen? Any new modifications, etc? Thanks :)
Ikezoe san ping...
Ikezoe san, can you please update the status... Thanks, Makuchaku
chen: *poke*
Not for 2.8.1. Will take it up for next release along with bug 264485.
The patch at the comment #23 has the similar side effects mentioned at http://bugzilla.gnome.org/show_bug.cgi?id=264485#c86. Regarding the patch at comment #14, makuchaku, please add a ChangeLog with the patch and it should not contain lines commented unless they are really required. If they are not required it should be removed. Please attach an updated patch. Thanks for being patient.
Since a lot of time has passed & more patches have gone into evolution, re-initiating a discussion on viable patch for this bug. Attaching a working patch in next comment.
Created attachment 78678 [details] [review] Latest patch Re-initiating a review for this patch. Lets solve the problems in other bugs as separate patches. Thanks, Mayank
Created attachment 94270 [details] [review] Updated patch for latest Subversion trunk Here's a refreshed version of Mayank's patch in comment #36 that applies cleanly to current Subversion trunk. I'm not knowledgeable enough in this area to tell whether the patch fixes the problem. Can someone else have a look?
The patch seems to fix the issue. Please commit the same.
Committed to SVN trunk as r34809 (http://svn.gnome.org/viewvc/evolution?view=revision&revision=34809)
Closing as FIXED.