Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow system language #2781

Merged
merged 4 commits into from
Feb 14, 2020
Merged

Follow system language #2781

merged 4 commits into from
Feb 14, 2020

Conversation

keianhzo
Copy link
Collaborator

@keianhzo keianhzo commented Feb 12, 2020

This adds support for a "Follow system language" placeholder that uses whatever the system locale is as long as it's supported by FxR, otherwise it falls back to en-US.

Also fixes some issues with the correct language hot reload. Now we should be able to change the language either from the Settings or from the system settings and everything so be instantly localized in the new language.

Things to test:

  • Change the language from the language setting: All the UI strings should be now localized in the new language
  • Click home - Go to the system settings - Change the language - Go back to the app: All the UI strings should be now localized in the new language
  • Reset the languages: The UI should show "Follow the system Language" and all the strings should be localized in the system language. In case of an unsupported language (ie. pt-PT), it should fall back to en-US
  • Set a display language explicitly - Click home - Go to the system settings - Change the language - Go back to the app: The user chosen language should remain as the UI language.
  • Check that when "Follow system language" is selected the voice search is performed in the correct language based on the system one. In case of an unsupported language (ie. pt-PT), it should fall back to en-US
  • Test updating from v8. The app should retain the previous language configuration and show specific locales for all three language settings.
  • All the sumo pages should be shown localized in the content language order. They were previously hardcoded to en-US.

@keianhzo keianhzo self-assigned this Feb 12, 2020
@keianhzo keianhzo added this to the #9 polish milestone Feb 12, 2020
@keianhzo keianhzo added the QA Attention QA label Feb 12, 2020
@keianhzo
Copy link
Collaborator Author

@Softvision-GeluHaiduc You can start with the language testing after this lands.

@bluemarvin
Copy link
Contributor

Got this crash on first run:

E/GeckoCrashHandler: >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
    java.lang.NullPointerException: Attempt to invoke interface method 'boolean java.util.List.isEmpty()' on a null object reference
        at org.mozilla.vrbrowser.utils.LocaleUtils.getPreferredLanguages(LocaleUtils.java:161)
        at org.mozilla.vrbrowser.utils.LocaleUtils.getPreferredLanguageTags(LocaleUtils.java:151)
        at org.mozilla.vrbrowser.VRBrowserActivity.onCreate(VRBrowserActivity.java:245)
        at android.app.Activity.performCreate(Activity.java:6724)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1119)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2632)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2740)
        at android.app.ActivityThread.-wrap12(ActivityThread.java)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1477)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:154)
        at android.app.ActivityThread.main(ActivityThread.java:6144)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

I get this crash on launch:

E/GeckoCrashHandler: >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
    java.lang.NullPointerException: Attempt to invoke interface method 'boolean java.util.List.isEmpty()' on a null object reference
        at org.mozilla.vrbrowser.utils.LocaleUtils.getPreferredLanguages(LocaleUtils.java:161)
        at org.mozilla.vrbrowser.utils.LocaleUtils.getPreferredLanguageTags(LocaleUtils.java:151)
        at org.mozilla.vrbrowser.VRBrowserActivity.onCreate(VRBrowserActivity.java:245)
        at android.app.Activity.performCreate(Activity.java:6724)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1119)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2632)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2740)
        at android.app.ActivityThread.-wrap12(ActivityThread.java)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1477)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:154)
        at android.app.ActivityThread.main(ActivityThread.java:6144)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

Did a clean build and got same result.

@bluemarvin
Copy link
Contributor

I get the same crash after a clean install also.

@bluemarvin
Copy link
Contributor

I was testing on the Quest.

@keianhzo
Copy link
Collaborator Author

Fixed.

Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

The telemetry tests failed for this PR. So we should probably figure that out or see if it was a one off failure.

@bluemarvin
Copy link
Contributor

@daoshengmu Said he would investigate the test failures.

@daoshengmu
Copy link
Contributor

It works with t-c now. :)

@MortimerGoro MortimerGoro merged commit 8a820da into master Feb 14, 2020
@MortimerGoro MortimerGoro deleted the v9/follow_system_language branch February 14, 2020 17:47
@Softvision-GeluHaiduc
Copy link

Softvision-GeluHaiduc commented Feb 17, 2020

I have verified these new behaviors using the latest RC, Firefox Reality version 9 RC1 (33f6c4d), installed on the Oculus Go, Quest, HTC Vive Focus, Focus Plus and Pico Neo2 headsets.
I have observed some different behaviors in the website language list on the HTC devices when French or Italian was set as app language and the placeholder for the URL bar doesn’t change languages when the device language is changed on the Pico Neo2. Also, the bookmark folders do not refresh after the app language is changed after a restart it displays the correct one.
In tested all the scenarios outlined in the PR and also some extra ones.

@Softvision-GeluHaiduc Softvision-GeluHaiduc added [QA]:Verified fixed Label for QA to mark verified fixed issues and removed QA Attention QA labels Feb 17, 2020
@keianhzo
Copy link
Collaborator Author

@Softvision-GeluHaiduc This two PRs should fix those issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[QA]:Verified fixed Label for QA to mark verified fixed issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants