After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693303 - Various patches to the keyboard status icon
Various patches to the keyboard status icon
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-07 09:54 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-02-07 10:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
keyboard: Fake out the readyCallback if we don't have IBus (874 bytes, patch)
2013-02-07 09:54 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
keyboard: Don't call inputSourcesChanged on init (1.18 KB, patch)
2013-02-07 09:54 UTC, Jasper St. Pierre (not reading bugmail)
rejected Details | Review
keyboard: Make sure to set currentSource if we only have one source (1.52 KB, patch)
2013-02-07 09:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
keyboard: Don't use set_skip_paint (1.34 KB, patch)
2013-02-07 09:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-02-07 09:54:21 UTC
The first two are sort of a cleanup -- I found that inputSourcesChanged
was getting called twice. Not that big of a deal, but seemed like we
should be able to do less.

When doing that, I tested to see how well the XKB-only path worked, and
found as far as I can tell the only issue with it. Nice work on that one.

The last one is a removal for set_skip_paint, which I want to trash soon.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-02-07 09:54:25 UTC
Created attachment 235352 [details] [review]
keyboard: Fake out the readyCallback if we don't have IBus

This allows us to move to one initialization code path if we only
have XKB.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-02-07 09:54:27 UTC
Created attachment 235353 [details] [review]
keyboard: Don't call inputSourcesChanged on init

IBusManager will call inputSourcesChanged when the engine is registered,
even if we don't have IBus.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-02-07 09:54:31 UTC
Created attachment 235354 [details] [review]
keyboard: Make sure to set currentSource if we only have one source

This way, popping up and re-closing the switcher won't emit an error
trying to check for this._currentSource.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-02-07 09:54:34 UTC
Created attachment 235355 [details] [review]
keyboard: Don't use set_skip_paint
Comment 5 Rui Matos 2013-02-07 10:14:15 UTC
(In reply to comment #0)
> The first two are sort of a cleanup -- I found that inputSourcesChanged
> was getting called twice. Not that big of a deal, but seemed like we
> should be able to do less.

You're claiming that it's being called twice when !IBus, i.e. when ibus is not even installed? I can't see how that could happen. Can you get stack traces?
Comment 6 Rui Matos 2013-02-07 10:16:48 UTC
Review of attachment 235353 [details] [review]:

If we do have ibus then the panel menu won't show up until we hear back from ibus-daemon which might take a while at session start.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-02-07 10:19:40 UTC
(In reply to comment #5)
> You're claiming that it's being called twice when !IBus, i.e. when ibus is not
> even installed? I can't see how that could happen. Can you get stack traces?

No, only when IBus is installed. There's once from the _init, and another from when the IBus engine starts up.
Comment 8 Rui Matos 2013-02-07 10:21:10 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > You're claiming that it's being called twice when !IBus, i.e. when ibus is not
> > even installed? I can't see how that could happen. Can you get stack traces?
> 
> No, only when IBus is installed. There's once from the _init, and another from
> when the IBus engine starts up.

Right, that's for reasons I explained in the patch review above.
Comment 9 Rui Matos 2013-02-07 10:27:12 UTC
Review of attachment 235354 [details] [review]:

++
Comment 10 Rui Matos 2013-02-07 10:37:41 UTC
Review of attachment 235355 [details] [review]:

Ok
Comment 11 Jasper St. Pierre (not reading bugmail) 2013-02-07 10:44:13 UTC
Attachment 235354 [details] pushed as 997f851 - keyboard: Make sure to set currentSource if we only have one source
Attachment 235355 [details] pushed as c0e5719 - keyboard: Don't use set_skip_paint