FIX: Using the `default_locale` in locale fallbacks caused problems

FIX: Using the default_locale in locale fallbacks caused problems

Locale files get precompiled after deployment and they contained translations from the default_locale. That’s especially bad in multisites, because the initial default_locale is en_US. Sites where the default_locale isn’t en_US could see missing translations. The same thing could happen when users are allowed to chose a different locale.

This change simplifies the logic by not using the default_locale in the locale chain. It always falls back to en in case of missing translations.

diff --git a/lib/i18n/backend/fallback_locale_list.rb b/lib/i18n/backend/fallback_locale_list.rb
index e1b3d25..f1b5aca 100644
--- a/lib/i18n/backend/fallback_locale_list.rb
+++ b/lib/i18n/backend/fallback_locale_list.rb
@@ -6,19 +6,15 @@ module I18n
     class FallbackLocaleList < Hash
       def [](locale)
         locale = locale.to_sym
-        return [locale] if locale == :en
+        locale_list = [locale]
+        return locale_list if locale == :en
 
-        fallback_locale = LocaleSiteSetting.fallback_locale(locale)
-        site_locale = SiteSetting.default_locale.to_sym
-
-        locale_list =
-          if locale == site_locale || site_locale == :en || fallback_locale == :en
-            [locale, fallback_locale, :en]
-          else
-            site_fallback_locale = LocaleSiteSetting.fallback_locale(site_locale)
-            [locale, fallback_locale, site_locale, site_fallback_locale, :en]
-          end
+        while (fallback_locale = LocaleSiteSetting.fallback_locale(locale))
+          locale_list << fallback_locale
+          locale = fallback_locale
+        end
 
+        locale_list << :en
         locale_list.uniq.compact
       end
     end
diff --git a/spec/components/js_locale_helper_spec.rb b/spec/components/js_locale_helper_spec.rb
index c1b9933..e1c9cb4 100644
--- a/spec/components/js_locale_helper_spec.rb
+++ b/spec/components/js_locale_helper_spec.rb
@@ -130,7 +130,7 @@ describe JsLocaleHelper do
     end
   end
 
-  it 'performs fallbacks to english if a translation is not available' do
+  it 'performs fallbacks to English if a translation is not available' do
     JsLocaleHelper.set_translations('en', "en" => {
         "js" => {
           "only_english" => "1-en",
@@ -161,12 +161,12 @@ describe JsLocaleHelper do
     expected = {
       "none" => "[uk.js.none]",
       "only_english" => "1-en",
-      "only_site" => "2-ru",
-      "english_and_site" => "3-ru",
+      "only_site" => "[uk.js.only_site]",
+      "english_and_site" => "3-en",
       "only_user" => "4-uk",
       "english_and_user" => "5-uk",
       "site_and_user" => "6-uk",
-      "all_three" => "7-uk",
+      "all_three" => "7-uk"
     }
 
     SiteSetting.default_locale = 'ru'
@@ -178,9 +178,9 @@ describe JsLocaleHelper do
     ctx.eval(JsLocaleHelper.output_locale(I18n.locale))
     ctx.eval('I18n.defaultLocale = "ru";')
 
-    expect(ctx.eval('I18n.translations.en.js').keys).to contain_exactly("only_english")
-    expect(ctx.eval('I18n.translations.ru.js').keys).to contain_exactly("only_site", "english_and_site")
+    expect(ctx.eval('I18n.translations').keys).to contain_exactly("uk", "en")
     expect(ctx.eval('I18n.translations.uk.js').keys).to contain_exactly("all_three", "english_and_user", "only_user", "site_and_user")
+    expect(ctx.eval('I18n.translations.en.js').keys).to contain_exactly("only_english", "english_and_site")
 
     expected.each do |key, expect|
       expect(ctx.eval("I18n.t(#{"js.#{key}".inspect})")).to eq(expect)
diff --git a/spec/lib/i18n/discourse_i18n_spec.rb b/spec/lib/i18n/discourse_i18n_spec.rb
index 0215dc3..39c0691 100644
--- a/spec/lib/i18n/discourse_i18n_spec.rb
+++ b/spec/lib/i18n/discourse_i18n_spec.rb
@@ -13,7 +13,6 @@ describe I18n::Backend::DiscourseI18n do
     backend.store_translations(:en, foo: 'Foo in :en', bar: 'Bar in :en', wat: 'Hello %{count}')
     backend.store_translations(:en, items: { one: 'one item', other: '%{count} items' })
     backend.store_translations(:de, bar: 'Bar in :de')
-    backend.store_translations(:ru, baz: 'Baz in :ru')
     backend.store_translations(:en, link: '[text](url)')
   end
 
@@ -54,13 +53,6 @@ describe I18n::Backend::DiscourseI18n do
       expect(backend.translate(:de, 'bar')).to eq('Bar in :de')
       expect(backend.translate(:de, 'foo')).to eq('Foo in :en')
     end
-
-    it 'uses default_locale as fallback when key exists' do
-      SiteSetting.default_locale = 'ru'
-      expect(backend.translate(:de, 'bar')).to eq('Bar in :de')
-      expect(backend.translate(:de, 'baz')).to eq('Baz in :ru')
-      expect(backend.translate(:de, 'foo')).to eq('Foo in :en')
-    end
   end
 
   describe '#exists?' do
@@ -75,7 +67,6 @@ describe I18n::Backend::DiscourseI18n do
     it 'returns true when an existing key and an existing locale is given' do
       expect(backend.exists?(:en, :foo)).to eq(true)
       expect(backend.exists?(:de, :bar)).to eq(true)
-      expect(backend.exists?(:ru, :baz)).to eq(true)
     end
 
     it 'returns false when a non-existing key and an existing locale is given' do
diff --git a/spec/lib/i18n/fallback_locale_list_spec.rb b/spec/lib/i18n/fallback_locale_list_spec.rb
index 793ac8b..f84a490 100644
--- a/spec/lib/i18n/fallback_locale_list_spec.rb
+++ b/spec/lib/i18n/fallback_locale_list_spec.rb
@@ -16,7 +16,7 @@ describe I18n::Backend::FallbackLocaleList do
   it "works when default_locale is English (United States)" do
     SiteSetting.default_locale = :en_US
 
-    expect(list[:ru]).to eq([:ru, :en_US, :en])
+    expect(list[:ru]).to eq([:ru, :en])
     expect(list[:en_US]).to eq([:en_US, :en])
     expect(list[:en]).to eq([:en])
   end
@@ -24,7 +24,7 @@ describe I18n::Backend::FallbackLocaleList do
   it "works when default_locale is not English" do
     SiteSetting.default_locale = :de
 
-    expect(list[:ru]).to eq([:ru, :de, :en])
+    expect(list[:ru]).to eq([:ru, :en])
     expect(list[:de]).to eq([:de, :en])
     expect(list[:en]).to eq([:en])
     expect(list[:en_US]).to eq([:en_US, :en])
@@ -34,6 +34,7 @@ describe I18n::Backend::FallbackLocaleList do
     before do
       DiscoursePluginRegistry.register_locale("es_MX", fallbackLocale: "es")
       DiscoursePluginRegistry.register_locale("de_AT", fallbackLocale: "de")
+      DiscoursePluginRegistry.register_locale("de_AT-formal", fallbackLocale: "de_AT")
     end
 
     after do
@@ -44,15 +45,27 @@ describe I18n::Backend::FallbackLocaleList do
       SiteSetting.default_locale = :en
 
       expect(list[:de_AT]).to eq([:de_AT, :de, :en])
+      expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
+      expect(list[:de]).to eq([:de, :en])
+      expect(list[:en]).to eq([:en])
+    end
+
+    it "works when default_locale is English (United States)" do
+      SiteSetting.default_locale = :en_US
+
+      expect(list[:de_AT]).to eq([:de_AT, :de, :en])
+      expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
       expect(list[:de]).to eq([:de, :en])
       expect(list[:en]).to eq([:en])
+      expect(list[:en_US]).to eq([:en_US, :en])
     end
 
     it "works when default_locale is not English" do
       SiteSetting.default_locale = :de
 
-      expect(list[:es_MX]).to eq([:es_MX, :es, :de, :en])
-      expect(list[:es]).to eq([:es, :de, :en])
+      expect(list[:es_MX]).to eq([:es_MX, :es, :en])
+      expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
+      expect(list[:es]).to eq([:es, :en])
       expect(list[:en]).to eq([:en])
     end
   end
diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb
index 570a861..fcc9153 100644
--- a/spec/models/theme_field_spec.rb
+++ b/spec/models/theme_field_spec.rb
@@ -8,6 +8,10 @@ describe ThemeField do
     ThemeField.destroy_all
   end
 
+  before do
+    I18n.locale = :en
+  end
+
   describe "scope: find_by_theme_ids" do
     it "returns result in the specified order" do
       theme = Fabricate(:theme)
@@ -60,22 +64,22 @@ describe ThemeField do
 
   it 'only extracts inline javascript to an external file' do
     html = <<~HTML
-    <script type="text/discourse-plugin" version="0.8">
-      var a = "inline discourse plugin";
-    </script>
-    <script type="text/template" data-template="custom-template">
-      <div>custom script type</div>
-    </script>
-    <script>
-      var b = "inline raw script";
-    </script>
-    <script type="texT/jAvasCripT">
-      var c = "text/javascript";
-    </script>

[... diff too long, it was truncated ...]

GitHub sha: ec2f3169