GNOME Bugzilla – Bug 603804
Mark unusable layouts as unusable
Last modified: 2009-12-05 16:36:07 UTC
If for some reason, your gconf config contains 5 layouts, then the keyboard capplet happily displays the 5 of them. Which is good. Also, it's not possible to add a new one. Which is also good. However, the 5th layout looks like all the other ones, and so the user probably thinks it should be working. Having a small hint that it's not usable (like making the label in the treeview not sensitive, or something like that) would help users understand that something is wrong and that the last layout cannot be used. If you ask me how the gconf key can contain 5 layout, then I'm pretty you can find some broken code somewhere that tries to play with that key...
Having 5 layouts in gconf means something went really wrong with the software that put it there (I do not mention manual tweak). The root cause should be fixed. As long as g-c-c and g-s-d do not crash, I think that's the best that can be expected.
Created attachment 149103 [details] [review] Simple patch I think it's really wrong to assume that only our tools will write to the gconf key, and since this is really a simple patch, it should go in (imho). It never hurts to not assume that we get valid data :-) Note that I think there's one boolean column in the list store that probably is wrong (the id column is not boolean, I'd guess). So even if the patch is rejected, this should get fixed.
(reopening so the patch can at least be considered)
I think any tools writing to gconf should know about assumptions we make. For example, the fact that storing layout zz(zz) would not work - because it is not in base.xml. But we're not validating layout/variant names, are we? Or should we display them in a special way as well? Of course I do not assume that we have valid data - otherwise the capplet would simply crash with 5 layouts;) But to put special code just to display invalid data - IMHO too much (or we should mark invalid layouts as well - to be consistent, right?) Regarding the boolean -> string column, that is well spotted bug, thanks. Actually, I do not even need 3 columns currently, so I will change that to 2. So, I am still inclined to have that closes as WONTFIX Actually, out of curiosity, what would be those "other tools"? May be, if you tell me more about them, I could reconsider...
(In reply to comment #4) > I think any tools writing to gconf should know about assumptions we make. For > example, the fact that storing layout zz(zz) would not work - because it is not > in base.xml. But we're not validating layout/variant names, are we? Or should > we display them in a special way as well? I'd say they should either be hidden or shown in a special way, yes. > Of course I do not assume that we have valid data - otherwise the capplet would > simply crash with 5 layouts;) But to put special code just to display invalid > data - IMHO too much (or we should mark invalid layouts as well - to be > consistent, right?) Well, yeah, that would be nice ;-) > Regarding the boolean -> string column, that is well spotted bug, thanks. > Actually, I do not even need 3 columns currently, so I will change that to 2. > > So, I am still inclined to have that closes as WONTFIX > > Actually, out of curiosity, what would be those "other tools"? May be, if you > tell me more about them, I could reconsider... I don't know of such tool, but I wouldn't be surprised that sysadmins have custom scripts for this. Anyway, it's up to you. My opinion is that the code is dead simple, so...
> I wouldn't be surprised that sysadmins have custom scripts for this. Really, never heard of those. I know some die-hards tweak xorg.conf/hal, use setxkbmap (at the session startup) etc. But I never heard of anyone who'd script kbd configuration through gconf. The code is really nice and simple indeed. But the idea behind it is wrong, IMO. Any decent xkb config tool/script should validate the number of layouts. Let's not cover, not decorate other people's faults and bugs... BTW, the semantics of "disabled" layouts is unclear from the GUI POV. It needs special explanation to user (hints or smth).
Sergey, when the fix is as simple as this, I think we should really consider properly filtering out unusable layouts, or at least showing that they won't work. I can understand not wanting to do that when it takes heaps of code to do, but when you can improve the situation in such a simple way, why the reluctance to do it? The GConf database is no ivory tower, so you always need to consider someone messing with it. You're right that the disabled semantics aren't clear, but neither is selecting the layout, and not having it work. At least when it's disabled I know something is wrong.
Ok, lads, you twisted my hands hard enough. I am not going to fight against two of you for 2 lines of code, so here you are. Committed. Even though I am still not excited about the idea... Vincent, out of curiosity - what was the cause of that patch? Did you encounter something in real life - some desktop with 5 layouts?
(In reply to comment #8) > Vincent, out of curiosity - what was the cause of that patch? Did you encounter > something in real life - some desktop with 5 layouts? Hrm, I think I had many layouts at some point while playing with the code. Or something like this.