GNOME Bugzilla – Bug 585259
Ignore invalid layouts in gconf
Last modified: 2009-06-11 13:51:49 UTC
For some reason, users might have some non-existing layout configured in gconf (in /desktop/gnome/peripherals/keyboard/kbd/layouts). At the moment, libgnomekbd blindly reads them and happily passes them to g-s-d. However, when g-s-d then tries to activate the configuration, an error appears. And the user gets the XKB error dialog at every startup. I think it'd be a good thing to have libgnomekbd at least ignore those layouts, and maybe even remove them from gconf.
I do not think it is a good idea. We should fight the root cause - these layouts appearing in gconf. The validation of layouts is non-trivial, generally speaking. We can validate against base.xml - but noone guarantees that some valid layout is present in that "directory". For example, there are some "compat" (old) layouts, which can appear in xorg.conf/gconf but they are not listed in base.xml. Do you have any idea why non-existing layouts appear in gconf at all?
(In reply to comment #1) > I do not think it is a good idea. We should fight the root cause - these > layouts appearing in gconf. The validation of layouts is non-trivial, generally > speaking. We can validate against base.xml - but noone guarantees that some > valid layout is present in that "directory". For example, there are some > "compat" (old) layouts, which can appear in xorg.conf/gconf but they are not > listed in base.xml. I would think that something like xkl_config_registry_find_layout() would be enough? > Do you have any idea why non-existing layouts appear in gconf at all? In the specific case I'm aware of, there was a gdm patch that made it use some invalid layout. This invalid layout was passed to the GNOME session via GDM_KEYBOARD_LAYOUT and since it was a first login, it was added to gconf. Arguably, in this case, we shouldn't blindly trust GDM_KEYBOARD_LAYOUT. Still, in the general case, a user won't be able to easily fix his gconf config once it's "corrupted" with an invalid layout.
> I would think that something like xkl_config_registry_find_layout() would be enough? Not exactly. Because it deals with base.xml. And, as I said, the negative answer does not necessarily mean the layout is invalid - it is just not mentioned in base.xml. >there was a gdm patch that made it use some invalid layout So, I guess that is the root cause. Actually GNOME has control over all situations where layouts go into gconf (except gconf-editor, which we do not have to support in that aspect, right?). So I think it is more sane not to "hide" broken layouts in gconf. But if that sad situation happen - it is better to indicate it (that will make user to report a bug, which is a good thing!). Of course, we should not crash - but sensible error message helping to indicate and resolve the issue - I guess that's the right thing to have. I really do not like the policy of hiding bugs under the carpet, if that's your intension. > a user won't be able to easily fix his gconf config once it's "corrupted" with an invalid layout. Ghm. Why not? Why cannot he go to g-k-p and choose something valid?
(In reply to comment #3) > >there was a gdm patch that made it use some invalid layout > So, I guess that is the root cause. Actually GNOME has control over all > situations where layouts go into gconf (except gconf-editor, which we do not > have to support in that aspect, right?). I wouldn't say that blindly trusting GDM_KEYBOARD_LAYOUT is something where we have real control ;-) But I can accept it's a corner case, sure. > So I think it is more sane not to > "hide" broken layouts in gconf. But if that sad situation happen - it is better > to indicate it (that will make user to report a bug, which is a good thing!). > Of course, we should not crash - but sensible error message helping to indicate > and resolve the issue - I guess that's the right thing to have. > > I really do not like the policy of hiding bugs under the carpet, if that's your > intension. It's certainly not my intention, but... > > a user won't be able to easily fix his gconf config once it's "corrupted" with an invalid layout. > Ghm. Why not? Why cannot he go to g-k-p and choose something valid? ... the current error dialog is probably not helpful to people. Maybe adding a button to launch g-k-p would make it better (each time I got a bug report about people seeing this dialog, the user never thought of launching g-k-p). But even with this, the user would still have to know which layout to remove. Still, I don't see how putting a dialog in front of the user is useful: it forces the user to deal with it, while, imho, it'd be better to just fix the issue (if we can detect what's wrong). This case is especially bad since it's a dialog on login, and not while interacting.
The solution found in discussion on IRC: check GDM_KEYBOARD_LAYOUT against base.xml (using xkl_config_registry_find_layout function), if it is there - put into gconf. If it is not there - ignore that envvar
Next idea: The dialog should be made after at least two attempts: first, try to activate "as is", second, try to remove all "unregistered" layouts/options
There is a performance (and memory) penalty here: loading base.xml by g-s-d... So, we have a serious question: is that usability issue worth the price?...
Finally(?): 1. Trust GDM envvar as it is now. 2. Implement "double-check" and load base.xml only if first attempt fails (in order to filter the configuration, dropping unregistered layouts). Gconf config remains untouched. If second attempt (using filtered configuration) succeeds, g-s-d proceeds silently.
Committed. Vincent, could you please check?
(In reply to comment #9) > Committed. Vincent, could you please check? Works great! Thanks for the magic!