GNOME Bugzilla – Bug 702344
common-language: fix languages in the user panel
Last modified: 2013-07-25 10:31:43 UTC
add_user_languages() was expected to add all languages currently in use in the system, but instead added only the default ones, meaning that the active one was sometimes missing.
Created attachment 246895 [details] [review] common-language: fix languages in the user panel
Review of attachment 246895 [details] [review]: The changes in get_lang_for_user_object_path() are only a clean-up to not instantiate GBusProxy instances, right? They should go a different patch then. ::: panels/common/cc-common-language.c @@ +743,2 @@ /* Add the current locale first */ name = cc_common_language_get_current_language (); Hmm, this call here is supposed to add the current language to the initial set. In fact get_user_languages() calls get_current_language() internally. More generally I'm not sure if we should change the behavior unconditionally, since there's the potential for the initial list to get too big while the design specifically calls for a small initial list.
(In reply to comment #2) > Review of attachment 246895 [details] [review]: > > The changes in get_lang_for_user_object_path() are only a clean-up to not > instantiate GBusProxy instances, right? They should go a different patch then. > > ::: panels/common/cc-common-language.c > @@ +743,2 @@ > /* Add the current locale first */ > name = cc_common_language_get_current_language (); > > Hmm, this call here is supposed to add the current language to the initial set. > In fact get_user_languages() calls get_current_language() internally. get_current_language(), as the name implies, returns the current language, without affecting external state. I don't see how it could add that to the initial set. > More generally I'm not sure if we should change the behavior unconditionally, > since there's the potential for the initial list to get too big while the > design specifically calls for a small initial list. I hope you're not implying that we should leave the user with an empty combobox in the user page just because it could get too big.
(In reply to comment #3) > (In reply to comment #2) > > Hmm, this call here is supposed to add the current language to the initial set. > > In fact get_user_languages() calls get_current_language() internally. > > get_current_language(), as the name implies, returns the current language, > without affecting external state. I don't see how it could add that to the > initial set. Ah, sorry, I misread. > > More generally I'm not sure if we should change the behavior unconditionally, > > since there's the potential for the initial list to get too big while the > > design specifically calls for a small initial list. > > I hope you're not implying that we should leave the user with an empty combobox > in the user page just because it could get too big. No I'm not. I'm saying that adding all the user accounts languages without a limit (say, at most 5) has the potential to make the list too big. Admittedly, this would be an highly unlikely scenario. Anyway, I'd say that we should just add the current users' language (of course, so that the combobox doesn't have an empty item) since we're already adding the "common" ones too and there are already 9 of those. For any further changes I'd like to hear a designer's opinion. Actually, to me, this combobox doesn't even make much sense since the language is something you configure in another panel already.
*** Bug 702168 has been marked as a duplicate of this bug. ***
Created attachment 247836 [details] [review] common: ensure the current language has a display name in the model This is a slightly simpler patch that just ensures the current language is present in the list of initial languages and therefore has a display name when added to the model.
Review of attachment 247836 [details] [review]: Didn't test but seems fine. Push to 3.8 too
Comment on attachment 247836 [details] [review] common: ensure the current language has a display name in the model Attachment 247836 [details] pushed as 0221ffe - common: ensure the current language has a display name in the model