[PATCH 0/2] Move all langs from geoip to top in Welcome spoke and fix default lang setting

[Martin Kolman]

The first patch fixes how default language is set and also cleans up the default language
setting code quite a bit.

The second patch moves all languages found for the given territory reported by GeoIP
to the top of the language list, above the separator, as suggested during the usability testing[1].
It also fixes locale variant preselection (so that for the GB territory, language is
set to English, but locale to English(UK)). And finally the patch cleans-up the locale and language
handling in Welcome spoke quite a bit.

[1] https://fedoraproject.org/wiki/Anaconda/UX_Redesign/Usability_Test_Suggestio...

Martin Kolman (2):
  Don't set ksdata.lang.seen to True if using default value
  Move all languages found by geoip to the top in Welcome spoke

anaconda                            |  3 +-
 pyanaconda/kickstart.py             |  5 ---
 pyanaconda/ui/gui/spokes/welcome.py | 70 +++++++++++++++++++++++++------------
 3 files changed, 49 insertions(+), 29 deletions(-)

[PATCH 1/2] Don't set ksdata.lang.seen to True if using default value

[Martin Kolman]

If setting kdsdata.lang.lang to the default value (DEFAULT_LANG),
don't set ksdata.lang.seen to True. Otherwise it would not be possible
to discern the default value from a value set by the user using
kickstart or a bootoption.

Also clean-up the language handling code a bit and remove
the default value setting in kickstart.py as it is no longer needed.

Related: rhbz#997397
Signed-off-by: Martin Kolman 
---
 anaconda                | 3 ++-
 pyanaconda/kickstart.py | 5 -----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/anaconda b/anaconda
index 3e745e3..c91ef2d 100755
--- a/anaconda
+++ b/anaconda
@@ -979,7 +979,8 @@ if __name__ == "__main__":
         else:
             log.error("Invalid locale '%s' given on command line or in kickstart", requested_lang)
     else:
-        localization.setup_locale(constants.DEFAULT_LANG)
+        # no kickstart or bootoption - use default
+        localization.setup_locale(constants.DEFAULT_LANG, ksdata.lang)

import blivet
     blivet.enable_installer_mode()
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index 3963889..3be0676 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -622,11 +622,6 @@ class IscsiName(commands.iscsiname.FC6_IscsiName):
         return retval

class Lang(commands.lang.F19_Lang):
-    def __init__(self, *args, **kwargs):
-        commands.lang.F19_Lang.__init__(self, *args, **kwargs)
-        if not self.lang and not flags.automatedInstall:
-            self.lang = DEFAULT_LANG
-
     def execute(self, *args, **kwargs):
         localization.write_language_configuration(self, ROOT_PATH)

[PATCH 1/2] Don't set ksdata.lang.seen to True if using default value

[Chris Lumens]

> If setting kdsdata.lang.lang to the default value (DEFAULT_LANG),
> don't set ksdata.lang.seen to True. Otherwise it would not be possible
> to discern the default value from a value set by the user using
> kickstart or a bootoption.
>
> Also clean-up the language handling code a bit and remove
> the default value setting in kickstart.py as it is no longer needed.

I don't see where you're avoiding setting ksdata.lang.seen.  Perhaps I
am missing something, but it seems like either the commit message is
wrong or there's a piece missing here.

- Chris

[PATCH 1/2] Don't set ksdata.lang.seen to True if using default value

[Vratislav Podzimek]

On Thu, 2013-09-12 at 11:08 -0400, Chris Lumens wrote:
> > If setting kdsdata.lang.lang to the default value (DEFAULT_LANG),
> > don't set ksdata.lang.seen to True. Otherwise it would not be possible
> > to discern the default value from a value set by the user using
> > kickstart or a bootoption.
> >
> > Also clean-up the language handling code a bit and remove
> > the default value setting in kickstart.py as it is no longer needed.
>
> I don't see where you're avoiding setting ksdata.lang.seen.  Perhaps I
> am missing something, but it seems like either the commit message is
> wrong or there's a piece missing here.

It would require a wider context of the patch to make it clearer.
Without this patch the pyanaconda/kickstart.py:Keyboard class sets its
default value (due to the text mode) which results in the code in the
anaconda script taking it as a value set in the kickstart (because it
doesn't check ksdata.lang.seen only ksdata.lang.lang), using it as a
requested value and setting ksdata.lang.seen to True.

With this patch, the class itself doesn't set its default value and the
code in the anaconda script goes the other path, just setting
ksdata.lang.lang to the defautl (due to the text mode) and not setting
ksdata.lang.seen to True, which fixes a serious bug that caused all
installations working as if the language was set to the default value in
kickstart.

[PATCH 2/2] Move all languages found by geoip to the top in Welcome spoke

[Martin Kolman]

There might be more languages used in a territory detected by GeoIP
and while anaconda preselects the most probable one, it has been
noted during the usability testing that even the less probable
languages should be also highlighted.

Therefore, all languages from geoip we have translations for
are now moved to the top of the language list in Welcome spoke,
above the separator. The most likely language is still on the top
of the list and is preselected.

Also for the given territory preselect the correct locale variant.
EXAMPLE: For the GB territory, the language should be preset to English
and the Locale to English(UK).

Signed-off-by: Martin Kolman 
---
 pyanaconda/ui/gui/spokes/welcome.py | 70 +++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py
index 79efdd2..7647d92 100644
--- a/pyanaconda/ui/gui/spokes/welcome.py
+++ b/pyanaconda/ui/gui/spokes/welcome.py
@@ -165,35 +165,59 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
         # to preselect the translation, when it's available.
         territory = geoloc.get_territory_code(wait=True)

-        locales = localization.get_territory_locales(territory)
-        if locales and not (self.data.lang.lang and self.data.lang.seen):
-            # get something from the GeoIP lookup and not set in/on the
-            # kickstart/command line
-            localization.setup_locale(locales[0], self.data.lang)
-
-        # Move the default language (whatever was provided on the command line,
-        # or by kickstart, or by geoip, or English if nothing else) to the top
-        # of the list and select it by default.  People find it confusing to be
-        # dropped into the middle of a scrollable list.
-        lang_itr, locale_itr = self._select_locale(self.data.lang.lang)
-
-        if not lang_itr or not locale_itr:
-            log.error("Failed to select language %s, using the default %s",
-                      self.data.lang.lang, DEFAULT_LANG)
-            lang_itr, locale_itr = self._select_locale(DEFAULT_LANG)
-            self.data.lang.lang = DEFAULT_LANG
+        # bootopts and kickstart have priority over geoip
+        if self.data.lang.lang and self.data.lang.seen:
+            locales = [self.data.lang.lang]
+        else:
+            locales = localization.get_territory_locales(territory) or [DEFAULT_LANG]

+        # get the data models
         filter_store = self._languageStoreFilter
-        # filtered store and lang_itr is an iter on it.  We need to
-        # convert to an iter on the underlying store.
-        itr = filter_store.convert_iter_to_child_iter(lang_itr)
         store = filter_store.get_model()
-        store.move_after(itr, None)

-        # And then we add a separator after the default chosen language.
-        newItr = store.insert(1)
+        # get language codes for the locales
+        langs = [localization.parse_langcode(locale)['language'] for locale in locales]
+
+        # check which of the geolocated languages have translations
+        # and store the iterators for those languages in a dictionary
+        langs_with_translations = {}
+        itr = store.get_iter_first()
+        while itr:
+            row_lang = store[itr][2]
+            if row_lang in langs:
+                langs_with_translations[row_lang] = itr
+            itr = store.iter_next(itr)
+
+        # if there are no translations for the given locales,
+        # use default
+        if not langs_with_translations:
+            localization.setup_locale(DEFAULT_LANG, self.data.lang)
+            lang_itr, locale_itr = self._select_locale(self.data.lang.lang)
+            langs_with_translations[DEFAULT_LANG] = lang_itr
+            locales = [DEFAULT_LANG]
+
+        # go over all geolocated languages in reverse order
+        # and move those we have translation for to the top of the
+        # list, above the separator
+        for lang in reversed(langs):
+            itr = langs_with_translations.get(lang)
+            if itr:
+                store.move_after(itr, None)
+            else:
+                # we don't have translation for this language,
+                # so dump all locales for it
+                locales = [l for l in locales
+                           if localization.parse_langcode(l)['language'] != lang]
+
+        # And then we add a separator after the selected best language
+        # and any additional languages (that have translations) from geoip
+        newItr = store.insert(len(langs_with_translations))
         store.set(newItr, 0, "", 1, "", 2, "", 3, True)

+        # setup the "best" locale
+        localization.setup_locale(locales[0], self.data.lang)
+        self._select_locale(self.data.lang.lang)
+
     def _retranslate_one(self, widgetName):
         widget = self.builder.get_object(widgetName)
         if not widget:

[PATCH 2/2] Move all languages found by geoip to the top in Welcome spoke

[Chris Lumens]

>    # if there are no translations for the given locales,
>    # use default
>    if not langs_with_translations:
>        localization.setup_locale(DEFAULT_LANG, self.data.lang)
>        lang_itr, locale_itr = self._select_locale(self.data.lang.lang)

locale_itr here is going to be an unused variable that pylint screams
about.

- Chris

[PATCH 2/2] Move all languages found by geoip to the top in Welcome spoke

[Martin Kolman]

On Thu, 2013-09-12 at 11:07 -0400, Chris Lumens wrote:
> >    # if there are no translations for the given locales,
> >    # use default
> >    if not langs_with_translations:
> >        localization.setup_locale(DEFAULT_LANG, self.data.lang)
> >        lang_itr, locale_itr = self._select_locale(self.data.lang.lang)
>
> locale_itr here is going to be an unused variable that pylint screams
> about.
> Chris
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

Oh, right, fixing locally. :)

[Vratislav Podzimek]

On Thu, 2013-09-12 at 12:55 +0200, Martin Kolman wrote:
> The first patch fixes how default language is set and also cleans up the default language
> setting code quite a bit.
>
> The second patch moves all languages found for the given territory reported by GeoIP
> to the top of the language list, above the separator, as suggested during the usability testing[1].
> It also fixes locale variant preselection (so that for the GB territory, language is
> set to English, but locale to English(UK)). And finally the patch cleans-up the locale and language
> handling in Welcome spoke quite a bit.
>
> [1] https://fedoraproject.org/wiki/Anaconda/UX_Redesign/Usability_Test_Suggestio...
>
> Martin Kolman (2):
>   Don't set ksdata.lang.seen to True if using default value
>   Move all languages found by geoip to the top in Welcome spoke
>
> anaconda                            |  3 +-
>  pyanaconda/kickstart.py             |  5 ---
>  pyanaconda/ui/gui/spokes/welcome.py | 70 +++++++++++++++++++++++++------------
>  3 files changed, 49 insertions(+), 29 deletions(-)

These both look good to me.