From 6fae74b36dd0109813730b3e8b284c8c01a39ce1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 17 Apr 2017 14:45:34 +0200 Subject: [PATCH 1/5] actually use phone's locale when picking descriptive metadata A TreeSet apparently does not really maintain insertion order, while a LinkedHashSet does. This ensures that the insertion order of locales is preserved in localesToUse so that the prioritization is correct. --- app/src/main/java/org/fdroid/fdroid/data/App.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 305ae9654..a9c421bcd 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -15,11 +15,9 @@ import android.os.Parcelable; import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; - import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; - import org.apache.commons.io.filefilter.RegexFileFilter; import org.fdroid.fdroid.AppFilter; import org.fdroid.fdroid.FDroidApp; @@ -40,10 +38,10 @@ import java.util.Collections; import java.util.Date; import java.util.Enumeration; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.TreeSet; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; @@ -377,7 +375,7 @@ public class App extends ValueObject implements Comparable, Parcelable { String languageTag = defaultLocale.getLanguage(); String localeTag = languageTag + "-" + defaultLocale.getCountry(); Set locales = localized.keySet(); - Set localesToUse = new TreeSet<>(); + Set localesToUse = new LinkedHashSet<>(); if (locales.contains(localeTag)) { localesToUse.add(localeTag); From 4bb7050725049dffd31cf37a84c3c1ecd7ffadb3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 17 Apr 2017 12:12:27 +0200 Subject: [PATCH 2/5] add Video to Links section of App Details --- app/src/main/java/org/fdroid/fdroid/data/App.java | 7 +++++-- .../fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java | 5 +++++ app/src/main/res/drawable/ic_video.xml | 7 +++++++ app/src/main/res/values/strings.xml | 1 + 4 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 app/src/main/res/drawable/ic_video.xml diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index a9c421bcd..066fa6296 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -396,11 +396,14 @@ public class App extends ValueObject implements Comparable, Parcelable { } } // if key starts with Upper case, its set by humans - video = getLocalizedEntry(localized, localesToUse, "Video"); + String value = getLocalizedEntry(localized, localesToUse, "Video"); + if (!TextUtils.isEmpty(value)) { + video = value.split("\n", 1)[0]; + } whatsNew = getLocalizedEntry(localized, localesToUse, "WhatsNew"); // Name, Summary, Description existed before localization so they shouldn't replace // non-localized old data format with a null or blank string - String value = getLocalizedEntry(localized, localesToUse, "Name"); + value = getLocalizedEntry(localized, localesToUse, "Name"); if (!TextUtils.isEmpty(value)) { name = value; } diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java index 5b0eed6bc..7cc73539a 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java @@ -740,6 +740,11 @@ public class AppDetailsRecyclerViewAdapter updateExpandableItem(false); contentView.removeAllViews(); + // Video link + if (uriIsSetAndCanBeOpened(app.video)) { + addLinkItemView(contentView, R.string.menu_video, R.drawable.ic_video, app.video); + } + // Source button if (uriIsSetAndCanBeOpened(app.sourceCode)) { addLinkItemView(contentView, R.string.menu_source, R.drawable.ic_source_code, app.sourceCode); diff --git a/app/src/main/res/drawable/ic_video.xml b/app/src/main/res/drawable/ic_video.xml new file mode 100644 index 000000000..e462ed24f --- /dev/null +++ b/app/src/main/res/drawable/ic_video.xml @@ -0,0 +1,7 @@ + + + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c42fd95a9..26f48dff5 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -159,6 +159,7 @@ E-Mail Author Issues Changelog + Video Source Code Upgrade Donate From 670e6be39a3621aabfd93e3f0eac7c3554b8cadd Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 17 Apr 2017 15:42:09 +0200 Subject: [PATCH 3/5] fix fallthrough setting of localized metadata entries Java's Map.get() returns null if there is no match, so this was always setting each entry to whatever value was in the highest priority locale, whether it had contents or what null. Now, this will fall through the priority list of locales until it finds actually contents. --- app/src/main/java/org/fdroid/fdroid/data/App.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 066fa6296..e1d46ea26 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -433,7 +433,10 @@ public class App extends ValueObject implements Comparable, Parcelable { try { for (String locale : locales) { if (localized.containsKey(locale)) { - return (String) localized.get(locale).get(key); + String value = (String) localized.get(locale).get(key); + if (value != null) { + return value; + } } } } catch (ClassCastException e) { From 3a194026fabbbd5e73888127666996b46fca39a1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 18 Apr 2017 20:32:51 +0200 Subject: [PATCH 4/5] test all formats in all languages We've had a number of crashes due to bad formats in various translated strings. This test runs through all of the translated strings and tests them with the same format values that the source strings expect. This is to ensure that the formats in the translations are correct in number and in type (e.g. {@code s} or {@code s}. It reads the source formats and then builds {@code formats} to represent the position and type of the formats. Then it runs through all of the translations with formats of the correct number and type. I couldn't get the Resources stuff working in Robolectric, so I made this an emulator test. The change to the Swedish translation included in this commit are fixes for issues that these tests found. closes #923 --- .../org/fdroid/fdroid/LocalizationTest.java | 210 ++++++++++++++++++ app/src/main/res/values-sv/strings.xml | 4 +- 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java diff --git a/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java b/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java new file mode 100644 index 000000000..5a38b2788 --- /dev/null +++ b/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java @@ -0,0 +1,210 @@ +package org.fdroid.fdroid; + +import android.app.Instrumentation; +import android.content.Context; +import android.content.res.AssetManager; +import android.content.res.Configuration; +import android.content.res.Resources; +import android.support.test.InstrumentationRegistry; +import android.support.test.runner.AndroidJUnit4; +import android.text.TextUtils; +import android.util.DisplayMetrics; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Runs through all of the translated strings and tests them with the same format + * values that the source strings expect. This is to ensure that the formats in + * the translations are correct in number and in type (e.g. {@code s} or {@code s}. + * It reads the source formats and then builds {@code formats} to represent the + * position and type of the formats. Then it runs through all of the translations + * with formats of the correct number and type. + */ +@RunWith(AndroidJUnit4.class) +public class LocalizationTest { + public static final String TAG = "LocalizationTest"; + + private final Pattern androidFormat = Pattern.compile("(%[a-z0-9]\\$?[a-z]?)"); + private final Locale[] locales = Locale.getAvailableLocales(); + private final ArrayList localeNames = new ArrayList<>(locales.length); + + private AssetManager assets; + private Configuration config; + private Resources resources; + + @Before + public void setUp() { + for (Locale locale : locales) { + localeNames.add(locale.toString()); + } + Collections.sort(localeNames); + + Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); + Context context = instrumentation.getTargetContext(); + assets = context.getAssets(); + config = context.getResources().getConfiguration(); + config.locale = Locale.ENGLISH; + // Resources() requires DisplayMetrics, but they are only needed for drawables + resources = new Resources(assets, new DisplayMetrics(), config); + } + + @Test + public void testLoadAllPlural() throws IllegalAccessException { + Field[] fields = R.plurals.class.getDeclaredFields(); + + HashMap haveFormats = new HashMap<>(); + for (Field field : fields) { + //Log.i(TAG, field.getName()); + int resId = field.getInt(int.class); + CharSequence string = resources.getQuantityText(resId, 4); + //Log.i(TAG, field.getName() + ": '" + string + "'"); + Matcher matcher = androidFormat.matcher(string); + int matches = 0; + char[] formats = new char[5]; + while (matcher.find()) { + String match = matcher.group(0); + char formatType = match.charAt(match.length() - 1); + switch (match.length()) { + case 2: + formats[matches] = formatType; + matches++; + break; + case 4: + formats[Integer.parseInt(match.substring(1, 2)) - 1] = formatType; + break; + case 5: + formats[Integer.parseInt(match.substring(1, 3)) - 1] = formatType; + break; + default: + throw new IllegalStateException(field.getName() + " has bad format: " + match); + } + } + haveFormats.put(field.getName(), new String(formats).trim()); + } + + for (Locale locale : locales) { + config.locale = locale; + // Resources() requires DisplayMetrics, but they are only needed for drawables + resources = new Resources(assets, new DisplayMetrics(), config); + for (Field field : fields) { + //Log.i(TAG, field.getName()); + int resId = field.getInt(int.class); + for (int quantity = 0; quantity < 22; quantity++) { + resources.getQuantityString(resId, quantity); + } + + String formats = haveFormats.get(field.getName()); + String formattedString = null; + switch (formats) { + case "d": + formattedString = resources.getQuantityString(resId, 1, 1); + break; + case "s": + formattedString = resources.getQuantityString(resId, 1, "ONE"); + break; + case "ds": + formattedString = resources.getQuantityString(resId, 2, 1, "TWO"); + break; + default: + if (!TextUtils.isEmpty(formats)) { + throw new IllegalStateException("Pattern not included in tests: " + formats); + } + } + if (formattedString != null) { // NOPMD + //Log.i(TAG, locale + " " + field.getName() + " FORMATTED: " + formattedString); + } + //Log.i(TAG, field.getName() + ": " + string); + } + } + } + + @Test + public void testLoadAllStrings() throws IllegalAccessException { + Field[] fields = R.string.class.getDeclaredFields(); + + HashMap haveFormats = new HashMap<>(); + for (Field field : fields) { + String string = resources.getString(field.getInt(int.class)); + Matcher matcher = androidFormat.matcher(string); + int matches = 0; + char[] formats = new char[5]; + while (matcher.find()) { + String match = matcher.group(0); + char formatType = match.charAt(match.length() - 1); + switch (match.length()) { + case 2: + formats[matches] = formatType; + matches++; + break; + case 4: + formats[Integer.parseInt(match.substring(1, 2)) - 1] = formatType; + break; + case 5: + formats[Integer.parseInt(match.substring(1, 3)) - 1] = formatType; + break; + default: + throw new IllegalStateException(field.getName() + " has bad format: " + match); + } + } + haveFormats.put(field.getName(), new String(formats).trim()); + } + + //for (Locale locale : new Locale[]{new Locale("es")}) { + for (Locale locale : locales) { + config.locale = locale; + // Resources() requires DisplayMetrics, but they are only needed for drawables + resources = new Resources(assets, new DisplayMetrics(), config); + for (Field field : fields) { + //Log.i(TAG, field.getName()); + int resId = field.getInt(int.class); + resources.getString(resId); + + String formats = haveFormats.get(field.getName()); + String formattedString = null; + switch (formats) { + case "d": + formattedString = resources.getString(resId, 1); + break; + case "dd": + formattedString = resources.getString(resId, 1, 2); + break; + case "s": + formattedString = resources.getString(resId, "ONE"); + break; + case "ss": + formattedString = resources.getString(resId, "ONE", "TWO"); + break; + case "sss": + formattedString = resources.getString(resId, "ONE", "TWO", "THREE"); + break; + case "ssss": + formattedString = resources.getString(resId, "ONE", "TWO", "THREE", "FOUR"); + break; + case "ssd": + formattedString = resources.getString(resId, "ONE", "TWO", 3); + break; + case "sssd": + formattedString = resources.getString(resId, "ONE", "TWO", "THREE", 4); + break; + default: + if (!TextUtils.isEmpty(formats)) { + throw new IllegalStateException("Pattern not included in tests: " + formats); + } + } + if (formattedString != null) { //NOPMD + //Log.i(TAG, "FORMATTED: " + formattedString); + } + //Log.i(TAG, field.getName() + ": " + string); + } + } + } +} diff --git a/app/src/main/res/values-sv/strings.xml b/app/src/main/res/values-sv/strings.xml index 4685e0e4c..72210ac38 100644 --- a/app/src/main/res/values-sv/strings.xml +++ b/app/src/main/res/values-sv/strings.xml @@ -481,8 +481,8 @@ Kategori %1$s - Visa enda appen i %2$d kategorin - Visa alla %2$d appar från %2$s kategorin + Visa enda appen i %2$s kategorin + Visa alla %1$d appar från %2$s kategorin Uppdatera From 67f40367cdd5b49b59a2b79acbada34790dd2be6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 18 Apr 2017 21:35:54 +0200 Subject: [PATCH 5/5] use Languages class as source of locales to test --- .../java/org/fdroid/fdroid/LocalizationTest.java | 9 +++++---- app/src/main/java/org/fdroid/fdroid/Languages.java | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java b/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java index 5a38b2788..d9e763b8e 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/LocalizationTest.java @@ -14,9 +14,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Locale; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -35,7 +34,7 @@ public class LocalizationTest { private final Pattern androidFormat = Pattern.compile("(%[a-z0-9]\\$?[a-z]?)"); private final Locale[] locales = Locale.getAvailableLocales(); - private final ArrayList localeNames = new ArrayList<>(locales.length); + private final HashSet localeNames = new HashSet<>(locales.length); private AssetManager assets; private Configuration config; @@ -43,10 +42,12 @@ public class LocalizationTest { @Before public void setUp() { + for (Locale locale : Languages.LOCALES_TO_TEST) { + localeNames.add(locale.toString()); + } for (Locale locale : locales) { localeNames.add(locale.toString()); } - Collections.sort(localeNames); Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); Context context = instrumentation.getTargetContext(); diff --git a/app/src/main/java/org/fdroid/fdroid/Languages.java b/app/src/main/java/org/fdroid/fdroid/Languages.java index ed5c572e6..ee9bb542b 100644 --- a/app/src/main/java/org/fdroid/fdroid/Languages.java +++ b/app/src/main/java/org/fdroid/fdroid/Languages.java @@ -203,7 +203,7 @@ public final class Languages { return Character.toUpperCase(line.charAt(0)) + line.substring(1); } - private static final Locale[] LOCALES_TO_TEST = { + public static final Locale[] LOCALES_TO_TEST = { Locale.ENGLISH, Locale.FRENCH, Locale.GERMAN,