Bug 796070

Summary: run ibus-setup produces IBUS-CRITICAL ** Traceback.
Product: [Fedora] Fedora Reporter: Lijun Li <lijli>
Component: ibusAssignee: fujiwara <tfujiwar>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: 17CC: aalam, dueno, i18n-bugs, lijli, shawn.p.huang, tfujiwar
Target Milestone: ---Keywords: i18n
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard:
Fixed In Version: ibus-fbterm-0.9.1-14.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-03-16 17:23:05 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
ibus-setup traceback
none
ibus-daemon --verbose
none
patch to fix ibus-setup crash when ibus-daemon is not running
none
ibus-setup works fine when ibus-daemon is running
none
ibus-daemon --verbose IBUS-WARNING none

Description Lijun Li 2012-02-22 03:30:05 EST
Description of problem:
run ibus-setup produces IBUS-CRITICAL ** Traceback.

Version-Release number of selected component (if applicable):
ibus-1.4.99.20120203-3.fc17.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Install Fedora17 Desktop from 
http://alt.fedoraproject.org/pub/alt/nightly-composes/
2. Select 'No Input Method' from Input Method Selector.
3. run ibus-setup from Terminal.
  
Actual results:
run ibus-setup produces IBUS-CRITICAL ** Traceback.

Expected results:
Should no traceback.

Additional info:
produce on en_US.
Comment 1 Lijun Li 2012-02-22 03:33:35 EST
Created attachment 564865 [details]
ibus-setup traceback
Comment 2 Lijun Li 2012-02-22 04:55:05 EST
Created attachment 564900 [details]
ibus-daemon --verbose
Comment 3 A S Alam 2012-02-22 05:00:32 EST
if run im-chooser and select ibus, then ibus-setup run properly, 
if run ibus-setup by disable ibus with im-chooser (No Input Method), then there is crash produced.
Comment 4 Daiki Ueno 2012-02-22 22:19:25 EST
Created attachment 565169 [details]
patch to fix ibus-setup crash when ibus-daemon is not running

Though maybe I guess fujiwara-san already knows the cause, I'm attaching a patch as I recently encountered a similar issue with ibus-setup-hangul gi port :)
Comment 5 fujiwara 2012-02-22 22:31:12 EST
(In reply to comment #4)
> Created attachment 565169 [details]
> patch to fix ibus-setup crash when ibus-daemon is not running
> 
> Though maybe I guess fujiwara-san already knows the cause, I'm attaching a
> patch as I recently encountered a similar issue with ibus-setup-hangul gi port
> :)

Thanks but maybe it's one of the issues due to attachment 564900 [details] through the yesterday's discussion.
Maybe need to check how %posttrans could work on readonly iso.
I will try the nightly iso today.
Comment 6 fujiwara 2012-02-22 22:57:43 EST
It seems I don't see any problems after I run ibus-daemon.
Do you have any problems after you run ibus-dameon?
Comment 7 Lijun Li 2012-02-23 00:46:23 EST
(In reply to comment #6)
> It seems I don't see any problems after I run ibus-daemon.
> Do you have any problems after you run ibus-dameon?

fujiwara,
That's right, there are not any problems after run ibus-dameon, I just Log Out and Log in the session to verify it, but for ibus-dameon --verbose still had the IBUS-WARNING message, and the second issue, run ibus-setup, the "IBus daemon is not started. Do you want to start it now?" dialog should prompt when ibus-daemon is not running. 

Thanks.
Comment 8 Lijun Li 2012-02-23 00:47:12 EST
Created attachment 565187 [details]
ibus-setup works fine when ibus-daemon is running
Comment 9 Lijun Li 2012-02-23 00:47:56 EST
Created attachment 565188 [details]
ibus-daemon --verbose IBUS-WARNING
Comment 10 fujiwara 2012-02-23 01:45:46 EST
OK, I understood your problem is only the case ibus-daemon doesn't run.
BTW, text attachments would be more useful for the log messages when you will file a bug next time.


What I was surprised yesterday is that the dconf key '[general:engines_order]' does not exist in the attachment 564900 [details] but it seems a small bug and ibus dconf db exists correctly.
Comment 11 fujiwara 2012-02-23 04:22:41 EST
--- ibus-phuang/setup/main.py.orig
+++ ibus-phuang/setup/main.py
@@ -338,9 +338,11 @@ class Setup(object):
             # self.__bus.config_add_watch("/general/hotkey")
             # self.__bus.config_add_watch("/panel")
         except:
-            while self.__bus == None:
+            pass
+        finally:
+            while self.__bus == None or not self.__bus.is_connected():
                 message = _("IBus daemon is not started. Do you want to start it now?")
-                dlg = Gtk.MessageDialog(type = Gtk.MESSAGE_QUESTION,
+                dlg = Gtk.MessageDialog(type = Gtk.MessageType.QUESTION,
                         buttons = Gtk.ButtonsType.YES_NO,
                         message_format = message)
                 id = dlg.run()
Comment 12 Daiki Ueno 2012-02-23 04:35:46 EST
(In reply to comment #11)
> +                dlg = Gtk.MessageDialog(type = Gtk.MessageType.QUESTION,
>                          buttons = Gtk.ButtonsType.YES_NO,
>                          message_format = message)
>                  id = dlg.run()

Have you tested the change?

There is also Gtk.MESSAGE_INFO below which should be Gtk.MessageType.INFO.
Also, it seems that self.__bus.destroy() is necessary to get self.__bus.is_connected() condition is True.
Comment 13 fujiwara 2012-02-23 20:24:33 EST
(In reply to comment #12)
> (In reply to comment #11)
> > +                dlg = Gtk.MessageDialog(type = Gtk.MessageType.QUESTION,
> >                          buttons = Gtk.ButtonsType.YES_NO,
> >                          message_format = message)
> >                  id = dlg.run()
> 
> Have you tested the change?

I think this part is ok.

> 
> There is also Gtk.MESSAGE_INFO below which should be Gtk.MessageType.INFO.
> Also, it seems that self.__bus.destroy() is necessary to get
> self.__bus.is_connected() condition is True.

Yes, I noticed Gtk.MESSAGE_INFO after I updated this yesterday. But no self.__bus.destroy() is ok in my test.

--- ibus-phuang/setup/main.py.orig
+++ ibus-phuang/setup/main.py
@@ -338,9 +338,11 @@ class Setup(object):
             # self.__bus.config_add_watch("/general/hotkey")
             # self.__bus.config_add_watch("/panel")
         except:
-            while self.__bus == None:
+            pass
+        finally:
+            while self.__bus == None or not self.__bus.is_connected():
                 message = _("IBus daemon is not started. Do you want to start it now?")
-                dlg = Gtk.MessageDialog(type = Gtk.MESSAGE_QUESTION,
+                dlg = Gtk.MessageDialog(type = Gtk.MessageType.QUESTION,
                         buttons = Gtk.ButtonsType.YES_NO,
                         message_format = message)
                 id = dlg.run()
@@ -351,6 +353,8 @@ class Setup(object):
                 pid = os.spawnlp(os.P_NOWAIT, "ibus-daemon", "ibus-daemon", "--xim")
                 time.sleep(1)
                 try:
+                    if self.__bus != None:
+                        self.__bus.destroy()
                     self.__bus = IBus.Bus()
                 except:
                     continue
@@ -360,7 +364,7 @@ class Setup(object):
                     "  export XMODIFIERS=@im=ibus\n"
                     "  export QT_IM_MODULE=ibus"
                     )
-                dlg = Gtk.MessageDialog(type = Gtk.MESSAGE_INFO,
+                dlg = Gtk.MessageDialog(type = Gtk.MessageType.INFO,
                                         buttons = Gtk.ButtonsType.OK,
                                         message_format = message)
                 id = dlg.run()
Comment 14 Daiki Ueno 2012-02-23 20:36:05 EST
(In reply to comment #13)

> Yes, I noticed Gtk.MESSAGE_INFO after I updated this yesterday. But no
> self.__bus.destroy() is ok in my test. 

IBusBus is actually a singleton (see "_bus" variable in ibusbus.c) and need to be destroyed to trigger reconnect.

>                  try:
> +                    if self.__bus != None:
> +                        self.__bus.destroy()
>                      self.__bus = IBus.Bus()
>                  except:
>                      continue

Hmm, this part still looks wrong.  Since IBus.Bus() does not raise any exception when bus is not connected, you need to check self.__bus.is_connected() and continue if it returns False.  Otherwise you will see a successful message dialog even if ibus-daemon fails to start.

try:
    if self.__bus != None:
        self.__bus.destroy()
    self.__bus = IBus.Bus()
    if not self.__bus.is_connected():
        continue
except:
    continue
Comment 15 fujiwara 2012-02-23 22:31:24 EST
(In reply to comment #14)
> Hmm, this part still looks wrong.  Since IBus.Bus() does not raise any
> exception when bus is not connected, you need to check
> self.__bus.is_connected() and continue if it returns False.  Otherwise you will
> see a successful message dialog even if ibus-daemon fails to start.
> 
> try:
>     if self.__bus != None:
>         self.__bus.destroy()
>     self.__bus = IBus.Bus()
>     if not self.__bus.is_connected():
>         continue
> except:
>     continue

OK, revised.

--- ibus-phuang/setup/main.py.orig
+++ ibus-phuang/setup/main.py
@@ -338,9 +338,13 @@ class Setup(object):
             # self.__bus.config_add_watch("/general/hotkey")
             # self.__bus.config_add_watch("/panel")
         except:
-            while self.__bus == None:
+            pass
+        finally:
+            if self.__bus != None and self.__bus.is_connected():
+                return
+            while self.__bus == None or not self.__bus.is_connected():
                 message = _("IBus daemon is not started. Do you want to start it now?")
-                dlg = Gtk.MessageDialog(type = Gtk.MESSAGE_QUESTION,
+                dlg = Gtk.MessageDialog(type = Gtk.MessageType.QUESTION,
                         buttons = Gtk.ButtonsType.YES_NO,
                         message_format = message)
                 id = dlg.run()
@@ -351,21 +355,23 @@ class Setup(object):
                 pid = os.spawnlp(os.P_NOWAIT, "ibus-daemon", "ibus-daemon", "--xim")
                 time.sleep(1)
                 try:
+                    if self.__bus != None:
+                        self.__bus.destroy()
                     self.__bus = IBus.Bus()
                 except:
                     continue
-                message = _("IBus has been started! "
-                    "If you can not use IBus, please add below lines in $HOME/.bashrc, and relogin your desktop.\n"
-                    "  export GTK_IM_MODULE=ibus\n"
-                    "  export XMODIFIERS=@im=ibus\n"
-                    "  export QT_IM_MODULE=ibus"
-                    )
-                dlg = Gtk.MessageDialog(type = Gtk.MESSAGE_INFO,
-                                        buttons = Gtk.ButtonsType.OK,
-                                        message_format = message)
-                id = dlg.run()
-                dlg.destroy()
-                self.__flush_gtk_events()
+            message = _("IBus has been started! "
+                "If you can not use IBus, please add below lines in $HOME/.bashrc, and relogin your desktop.\n"
+                "  export GTK_IM_MODULE=ibus\n"
+                "  export XMODIFIERS=@im=ibus\n"
+                "  export QT_IM_MODULE=ibus"
+                )
+            dlg = Gtk.MessageDialog(type = Gtk.MessageType.INFO,
+                                    buttons = Gtk.ButtonsType.OK,
+                                    message_format = message)
+            id = dlg.run()
+            dlg.destroy()
+            self.__flush_gtk_events()
 
     def __shortcut_button_clicked_cb(self, button, name, section, _name, entry):
         buttons = (Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL,
Comment 16 Daiki Ueno 2012-02-24 18:33:32 EST
(In reply to comment #15)
> > Hmm, this part still looks wrong.  Since IBus.Bus() does not raise any
> > exception when bus is not connected, you need to check
> > self.__bus.is_connected() and continue if it returns False.  Otherwise you will
> > see a successful message dialog even if ibus-daemon fails to start.

> OK, revised.
> 

>                  try:
> +                    if self.__bus != None:
> +                        self.__bus.destroy()
>                      self.__bus = IBus.Bus()
>                  except:
>                      continue

It has not changed from the previous version?  Anyway, I would suggest that you should test it at least.  That shouldn't be hard I guess.  Though this bug sounds not that important for you, I wonder if it isn't the case of the final release criteria:

https://fedoraproject.org/wiki/Fedora_17_Final_Release_Criteria

* All applications listed under the Applications menu or category must start successfully
Comment 17 fujiwara 2012-02-26 20:22:50 EST
(In reply to comment #16)
> (In reply to comment #15)
> > > Hmm, this part still looks wrong.  Since IBus.Bus() does not raise any
> > > exception when bus is not connected, you need to check
> > > self.__bus.is_connected() and continue if it returns False.  Otherwise you will
> > > see a successful message dialog even if ibus-daemon fails to start.
> 
> > OK, revised.
> > 
> 
> >                  try:
> > +                    if self.__bus != None:
> > +                        self.__bus.destroy()
> >                      self.__bus = IBus.Bus()
> >                  except:
> >                      continue
> 
> It has not changed from the previous version?  Anyway, I would suggest that you

This was fixed in the previous one.
I also suggest to close your cl on upstream by yourself and I will update it.
Comment 18 Daiki Ueno 2012-03-01 00:17:29 EST
(In reply to comment #17)
> > It has not changed from the previous version?
> This was fixed in the previous one.

No, the following check which I pointed out in comment #14 is missing (in neither the version posted here nor your upstream cl):

    if not self.__bus.is_connected():
        continue

You can easily reproduce the wrong behavior with:

1. stop ibus-daemon
2. sudo chmod -x /usr/bin/ibus-daemon
3. ibus-setup

Then you will see "IBus has been started! ..." message.

> I also suggest to close your cl on upstream by yourself and I will update it.

Just filed: https://codereview.appspot.com/5716045/
Comment 19 fujiwara 2012-03-01 02:23:56 EST
(In reply to comment #18)
> (In reply to comment #17)
> > > It has not changed from the previous version?
> > This was fixed in the previous one.
> 
> No, the following check which I pointed out in comment #14 is missing (in
> neither the version posted here nor your upstream cl):
> 
>     if not self.__bus.is_connected():
>         continue
> 
> You can easily reproduce the wrong behavior with:
> 
> 1. stop ibus-daemon
> 2. sudo chmod -x /usr/bin/ibus-daemon
> 3. ibus-setup
> 
> Then you will see "IBus has been started! ..." message.

I don't see any problems.

> 
> > I also suggest to close your cl on upstream by yourself and I will update it.
> 
> Just filed: https://codereview.appspot.com/5716045/

Not sure why you're working on this while I accept this.
Comment 20 Daiki Ueno 2012-03-01 02:53:42 EST
(In reply to comment #19)
> I don't see any problems.

Ah!  You are right, I didn't notice the indentation change that moved the code showing the successful dialog out of the loop.  Sorry for the noise.

> > > I also suggest to close your cl on upstream by yourself and I will update it.
> > Just filed: https://codereview.appspot.com/5716045/
> 
> Not sure why you're working on this while I accept this.

? perhaps I didn't understand your suggestion above.
Comment 21 fujiwara 2012-03-01 03:25:48 EST
I think your changes are not important at all.
Personally I feel you just try to push your motivations instead of the bug fixes or feature implementations.
Anyway I think your work is out of this issue and I don't like to review that again.
Comment 22 Daiki Ueno 2012-03-04 03:21:50 EST
(In reply to comment #21)
> I think your changes are not important at all.
> Personally I feel you just try to push your motivations instead of the bug
> fixes or feature implementations.
> Anyway I think your work is out of this issue and I don't like to review that
> again.

I would apologize if you feel like I infringed on your play ground.  However, I can't understand why you started working on your own patch before reviewing my patch.  It didn't contain any flaw that I pointed out in comment #12.

Anyway, I will file a bug at upstream first instead of the bugzilla from the next time.
Comment 23 fujiwara 2012-03-04 21:48:05 EST
(In reply to comment #22)
> (In reply to comment #21)
> > I think your changes are not important at all.
> > Personally I feel you just try to push your motivations instead of the bug
> > fixes or feature implementations.
> > Anyway I think your work is out of this issue and I don't like to review that
> > again.
> 
> I would apologize if you feel like I infringed on your play ground.  However, I
> can't understand why you started working on your own patch before reviewing my
> patch.  It didn't contain any flaw that I pointed out in comment #12.

That was the example fix and I didn't say it's finished and I don't think your patch is good for this easy fix.

> Anyway, I will file a bug at upstream first instead of the bugzilla from the
> next time.
Comment 24 Fedora Update System 2012-03-13 07:17:57 EDT
ibus-fbterm-0.9.1-14.fc17, ibus-qt-1.3.1-8.fc17, ibus-handwrite-2.1.4-4.fc17, sunpinyin-2.0.3-4.fc17, ibus-input-pad-1.4.0-7.fc17, eekboard-1.0.5-4.fc17, ibus-skk-1.4.0-3.fc17, ibus-sayura-1.3.1-4.fc17, ibus-rawcode-1.3.1.20100707-7.fc17, ibus-m17n-1.3.3-9.fc17, ibus-hangul-1.4.0-5.fc17, ibus-chewing-1.3.10-3.fc17, ibus-pinyin-1.4.0-14.fc17, ibus-1.4.99.20120304-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/FEDORA-2012-3532/ibus-fbterm-0.9.1-14.fc17,ibus-qt-1.3.1-8.fc17,ibus-handwrite-2.1.4-4.fc17,sunpinyin-2.0.3-4.fc17,ibus-input-pad-1.4.0-7.fc17,eekboard-1.0.5-4.fc17,ibus-skk-1.4.0-3.fc17,ibus-sayura-1.3.1-4.fc17,ibus-rawcode-1.3.1.20100707-7.fc17,ibus-m17n-1.3.3-9.fc17,ibus-hangul-1.4.0-5.fc17,ibus-chewing-1.3.10-3.fc17,ibus-pinyin-1.4.0-14.fc17,ibus-1.4.99.20120304-3.fc17
Comment 25 Fedora Update System 2012-03-13 13:09:02 EDT
Package ibus-fbterm-0.9.1-14.fc17, ibus-qt-1.3.1-8.fc17, ibus-handwrite-2.1.4-4.fc17, sunpinyin-2.0.3-4.fc17, ibus-input-pad-1.4.0-7.fc17, eekboard-1.0.5-4.fc17, ibus-skk-1.4.0-3.fc17, ibus-sayura-1.3.1-4.fc17, ibus-rawcode-1.3.1.20100707-7.fc17, ibus-m17n-1.3.3-9.fc17, ibus-hangul-1.4.0-5.fc17, ibus-chewing-1.3.10-3.fc17, ibus-pinyin-1.4.0-14.fc17, ibus-1.4.99.20120304-3.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ibus-fbterm-0.9.1-14.fc17 ibus-qt-1.3.1-8.fc17 ibus-handwrite-2.1.4-4.fc17 sunpinyin-2.0.3-4.fc17 ibus-input-pad-1.4.0-7.fc17 eekboard-1.0.5-4.fc17 ibus-skk-1.4.0-3.fc17 ibus-sayura-1.3.1-4.fc17 ibus-rawcode-1.3.1.20100707-7.fc17 ibus-m17n-1.3.3-9.fc17 ibus-hangul-1.4.0-5.fc17 ibus-chewing-1.3.10-3.fc17 ibus-pinyin-1.4.0-14.fc17 ibus-1.4.99.20120304-3.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-3532/ibus-fbterm-0.9.1-14.fc17,ibus-qt-1.3.1-8.fc17,ibus-handwrite-2.1.4-4.fc17,sunpinyin-2.0.3-4.fc17,ibus-input-pad-1.4.0-7.fc17,eekboard-1.0.5-4.fc17,ibus-skk-1.4.0-3.fc17,ibus-sayura-1.3.1-4.fc17,ibus-rawcode-1.3.1.20100707-7.fc17,ibus-m17n-1.3.3-9.fc17,ibus-hangul-1.4.0-5.fc17,ibus-chewing-1.3.10-3.fc17,ibus-pinyin-1.4.0-14.fc17,ibus-1.4.99.20120304-3.fc17
then log in and leave karma (feedback).
Comment 26 Fedora Update System 2012-03-16 17:23:05 EDT
ibus-fbterm-0.9.1-14.fc17, ibus-qt-1.3.1-8.fc17, ibus-handwrite-2.1.4-4.fc17, sunpinyin-2.0.3-4.fc17, ibus-input-pad-1.4.0-7.fc17, eekboard-1.0.5-4.fc17, ibus-skk-1.4.0-3.fc17, ibus-sayura-1.3.1-4.fc17, ibus-rawcode-1.3.1.20100707-7.fc17, ibus-m17n-1.3.3-9.fc17, ibus-hangul-1.4.0-5.fc17, ibus-chewing-1.3.10-3.fc17, ibus-pinyin-1.4.0-14.fc17, ibus-1.4.99.20120304-3.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.