Skip to content
4 changes: 4 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ android {
androidTest.assets.srcDirs += files("$projectDir/schemas".toString())
}

androidResources {
generateLocaleConfig = true
}

Comment on lines +100 to +103
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't pay attention to new languages being added from Weblate, this might lead to half-translated being proposed to the user. Currently the list of languages is manually kept in settings_keys.xml, so when new languages are added through Weblate they don't automatically appear in the app menu. However, I'm fine with switching to the new behavior, as it would avoid work to keep the language list up to date. Is it possible to access the locale config from within the app, so we can delete the list of languages in settings_keys.xml and use the autogenerated locale config even for the pre-Android-13 language picker?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found LocaleManager#getOverrideLocaleConfig but it sounds like that method is only to get a LocaleConfig that's already been set with setOverrideLocaleConfig(). As far as I can tell, we would have to manually generate a LocaleConfig instead of using the auto-generated one if we wanted to use it for this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could find the data under app/build/generated/res/localeConfig/debug/xml/_generated_res_locale_config.xml after running the build, with this data inside:

<locale-config xmlns:android="http://schemas.android.com/apk/res/android">
    <locale android:name="en-US"/>
    <locale android:name="ace"/>
    ...

So in theory the data is there, but in practice I think we better not access it by manually opening the xml since it's not officially supported. So yeah let's keep the previous behavior.

buildFeatures {
viewBinding true
buildConfig true
Expand Down
26 changes: 26 additions & 0 deletions app/src/main/java/org/schabi/newpipe/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
import android.app.Application;
import android.content.Context;
import android.content.SharedPreferences;
import android.os.Build;
import android.util.Log;

import androidx.annotation.NonNull;
import androidx.appcompat.app.AppCompatDelegate;
import androidx.core.app.NotificationChannelCompat;
import androidx.core.app.NotificationManagerCompat;
import androidx.core.os.LocaleListCompat;
import androidx.preference.PreferenceManager;

import com.jakewharton.processphoenix.ProcessPhoenix;
Expand Down Expand Up @@ -122,6 +125,29 @@ public void onCreate() {
configureRxJavaErrorHandler();

YoutubeStreamExtractor.setPoTokenProvider(PoTokenProviderImpl.INSTANCE);

if (Build.VERSION.SDK_INT >= 33) {
ensureAppLanguagePreferenceIsMigrated(prefs);
}
}

private void ensureAppLanguagePreferenceIsMigrated(final SharedPreferences prefs) {
final String appLanguageDefaultValue = getString(R.string.default_localization_key);
final String appLanguageKey = getString(R.string.app_language_key);
final String appLanguageCurrentValue = prefs.getString(appLanguageKey, null);
if (appLanguageCurrentValue != null) {
// Migrate to Android per-app language settings
prefs.edit().remove(appLanguageKey).apply();
if (!appLanguageCurrentValue.equals(appLanguageDefaultValue)) {
try {
AppCompatDelegate.setApplicationLocales(
LocaleListCompat.forLanguageTags(appLanguageCurrentValue)
);
} catch (final RuntimeException e) {
Log.e(TAG, "Error migrating to Android 13+ per-app language settings");
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like it should be moved and converted to a Migration in org.schabi.newpipe.settings.SettingMigrations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is that if we convert this to a Migration, my understanding is that it will only run once. So in theory, the following scenario is possible:

  1. Someone running NewPipe on Android <13 sets a custom app language
  2. They update to the new version of NewPipe, where this migration is run but doesn't do anything because they're on Android <13
  3. They update to Android 13+
  4. Their custom app language pref isn't used (because it's running the Android 13+ code path) and it's not migrated from their old preference (because there's nothing triggering the migration to run again)

I recognize this will probably be a pretty rare scenario, just wanted to flag it. But whether or not we make it a Migration, I do recognize that this code belongs somewhere outside of the Application subclass for sure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved it to a Migration. If we want to prevent the hypothetical app language setting loss I described above, maybe we could add a new migrations file for "migrations that should always be run on app startup because they depend on the current Android version".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmh you are right, this shouldn't be a migration as it needs to run every time the app notices it has been updated to Android 13+ from a lower Android version. What do you think @TobiGr?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. My bad. I'd move it to initSettings in NewPipeSettings then. Does that make sense @Stypox ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package org.schabi.newpipe.settings;

import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.provider.Settings;
import android.util.Log;
import android.widget.Toast;

import androidx.appcompat.app.AppCompatDelegate;
import androidx.preference.Preference;

import org.schabi.newpipe.DownloaderImpl;
Expand All @@ -17,6 +22,7 @@
import org.schabi.newpipe.util.image.PreferredImageQuality;

import java.io.IOException;
import java.util.Locale;

public class ContentSettingsFragment extends BasePreferenceFragment {
private String youtubeRestrictedModeEnabledKey;
Expand All @@ -37,6 +43,26 @@ public void onCreatePreferences(final Bundle savedInstanceState, final String ro
.getPreferredContentCountry(requireContext());
initialLanguage = defaultPreferences.getString(getString(R.string.app_language_key), "en");

if (Build.VERSION.SDK_INT >= 33) {
requirePreference(R.string.app_language_key).setVisible(false);
final Preference newAppLanguagePref =
requirePreference(R.string.app_language_android_13_and_up_key);
newAppLanguagePref.setSummaryProvider(preference -> {
final Locale customLocale = AppCompatDelegate.getApplicationLocales().get(0);
if (customLocale != null) {
return customLocale.getDisplayName();
}
return getString(R.string.systems_language);
});
newAppLanguagePref.setOnPreferenceClickListener(preference -> {
final Intent intent = new Intent(Settings.ACTION_APP_LOCALE_SETTINGS)
.setData(Uri.fromParts("package", requireContext().getPackageName(), null));
startActivity(intent);
Comment on lines +52 to +54
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very edge case, but if the user has disabled settings or if some ROMs are broken (hello weird Android TV devices), this would crash the app due to no app being able to handle the intent.

return true;
});
newAppLanguagePref.setVisible(true);
}

final Preference imageQualityPreference = requirePreference(R.string.image_quality_key);
imageQualityPreference.setOnPreferenceChangeListener(
(preference, newValue) -> {
Expand Down
6 changes: 6 additions & 0 deletions app/src/main/java/org/schabi/newpipe/util/Localization.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import androidx.annotation.Nullable;
import androidx.annotation.PluralsRes;
import androidx.annotation.StringRes;
import androidx.appcompat.app.AppCompatDelegate;
import androidx.core.math.MathUtils;
import androidx.preference.PreferenceManager;

Expand All @@ -39,6 +40,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.stream.Collectors;


Expand Down Expand Up @@ -101,6 +103,10 @@ public static Locale getPreferredLocale(@NonNull final Context context) {
}

public static Locale getAppLocale(@NonNull final Context context) {
if (Build.VERSION.SDK_INT >= 33) {
final Locale customLocale = AppCompatDelegate.getApplicationLocales().get(0);
return Objects.requireNonNullElseGet(customLocale, Locale::getDefault);
}
return getLocaleFromPrefs(context, R.string.app_language_key);
}

Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/resources.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
unqualifiedResLocale=en-US
1 change: 1 addition & 0 deletions app/src/main/res/values/settings_keys.xml
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@
<string name="playback_skip_silence_key">playback_skip_silence_key</string>

<string name="app_language_key">app_language_key</string>
<string name="app_language_android_13_and_up_key">app_language_android_13_and_up_key</string>

<string name="feed_update_threshold_key">feed_update_threshold_key</string>
<string name="feed_update_threshold_default_value">300</string>
Expand Down
7 changes: 7 additions & 0 deletions app/src/main/res/xml/content_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
app:iconSpaceReserved="false"
app:useSimpleSummaryProvider="true" />

<Preference
android:key="@string/app_language_android_13_and_up_key"
android:title="@string/app_language_title"
app:isPreferenceVisible="false"
app:singleLineTitle="false"
app:iconSpaceReserved="false" />
Comment on lines +16 to +21
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a separate Preference for the Android 13+ behavior only because for some reason when I set an OnPreferenceClickListener on the existing preference that returns true (indicating "the click was handled"), it still opens the existing dialog.


<ListPreference
android:defaultValue="@string/default_localization_key"
android:entries="@array/language_names"
Expand Down