Settings title remains as "Language" after switching locale and returning to "General Settings"

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ahunt, Assigned: dlim)

Tracking

({regression})

Trunk
Firefox 50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 wontfix, firefox46 wontfix, firefox47 wontfix, firefox48 fix-optional, firefox49 verified, fennec48+, firefox50 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

3 years ago
1. Open Settings
2. Go to General->Language
3. Select a new Language
   Note: title in the ActionBar switches to equivalent of "Language" in the new Locale
4. Press back (either in AB, or the system back button)
5. Title remains "Language" (or translated equivalent) but should be "General" (we're showing General settings correctly, just the title is wrong)

Could be a regression caused by Bug 1221679 ?
Reporter

Comment 1

3 years ago
I've tested on both an N4 and an N7 running nightly. Both of these use the phone-layout (single-pane) settings. I haven't tested on a real (multi-pane) tablet yet.
(In reply to Andrzej Hunt :ahunt from comment #0)

> Could be a regression caused by Bug 1221679 ?

Could very well be. This code is really tricky and difficult to work with.
tracking-fennec: --- → ?
regression:
good build: 11-11-2015
bad build: 12-11-2015

pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cc473fe5dc512c450634506f68cbacfb40a06a23&tochange=3cc3b1968524248450c465c4ea2ee5596ffa65f2

Bug 1221679 - Changing language on phone will update language settings page title with "General"
needs an owner
Flags: needinfo?(margaret.leibovic)
I caused this, so I'll look into it. However, I don't think this is too big of a problem, so it's okay that we're shipping this regression.
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → 48+
Flags: needinfo?(margaret.leibovic)
Assignee

Updated

3 years ago
Duplicate of this bug: 1274889
Assignee

Updated

3 years ago
Assignee: margaret.leibovic → dlim
Assignee

Comment 8

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57436/diff/1-2/
Attachment #8759449 - Flags: review?(gkruglov)

Comment 10

3 years ago
https://reviewboard.mozilla.org/r/57436/#review54482

Good first pass!

Check out the try results - there are some lint/checkstyle issues, and tests seem to be failing. I'll give it another look once those are resolved.

::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:285
(Diff revision 2)
>              if (onIsMultiPane()) {
>                  updateActionBarTitle(R.string.settings_title);
>              }
>  
>              // Update the title to for the preference pane that we're currently showing.
> -            setTitle(R.string.pref_category_language);
> +            int titleId = getIntent().getExtras().getInt(PreferenceActivity.EXTRA_SHOW_FRAGMENT_TITLE);

final whenever possible

::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:289
(Diff revision 2)
>              // Update the title to for the preference pane that we're currently showing.
> -            setTitle(R.string.pref_category_language);
> +            int titleId = getIntent().getExtras().getInt(PreferenceActivity.EXTRA_SHOW_FRAGMENT_TITLE);
> +            if (titleId != NO_SUCH_ID) {
> +                setTitle(titleId);
> +            } else {
> +                Log.e(LOGTAG, "Title id not found in intent bundle extras");

Should this ever happen? Perhaps we want to throw, to avoid getting into a weird state?

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/components/SettingsComponent.java:1
(Diff revision 2)
> +package org.mozilla.gecko.tests.components;

license

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/components/SettingsComponent.java:9
(Diff revision 2)
> +
> +import org.mozilla.gecko.R;
> +import org.mozilla.gecko.tests.UITestContext;
> +
> +/**
> + * Created by root on 6/2/16.

proper comment

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testPreference.java:1
(Diff revision 2)
> +package org.mozilla.gecko.tests;

Missing license. Use Public Domain; look at other tests in the tree, e.g. https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/android/VisitsHelperTest.java#1

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testPreference.java:13
(Diff revision 2)
> +import org.mozilla.gecko.tests.helpers.GeckoHelper;
> +
> +import static org.mozilla.gecko.tests.helpers.AssertionHelper.fAssertEquals;
> +
> +/**
> + * Created by root on 6/2/16.

Please provide a proper comment describing what this is testing.
Assignee

Comment 12

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57436/diff/2-3/
Assignee

Comment 13

3 years ago
Comment on attachment 8760437 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58006/diff/1-2/
Attachment #8760437 - Attachment description: Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings" → Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".
Attachment #8760437 - Flags: review?(gkruglov)
Assignee

Comment 14

3 years ago
(In reply to Daniel Lim [:dlim] from comment #13)
> Comment on attachment 8760437 [details]
> Bug 1263726 - Settings title remains as "Language" after switching locale
> and returning to "General Settings".
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/58006/diff/1-2/

Removed robocop test due to try-server not being able to do multilocale build resulting in test failure.
Assignee

Comment 17

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57436/diff/3-4/
Assignee

Updated

3 years ago
Attachment #8760437 - Attachment is obsolete: true
Attachment #8760437 - Flags: review?(gkruglov)
Assignee

Comment 18

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57436/diff/4-5/
Assignee

Comment 19

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57436/diff/5-6/

Updated

3 years ago
Attachment #8759449 - Flags: review?(gkruglov) → review+

Comment 20

3 years ago
Comment on attachment 8759449 [details]
Bug 1263726 - Settings title remains as "Language" after switching locale and returning to "General Settings".

https://reviewboard.mozilla.org/r/57436/#review54962

Nice, thanks dlim! Feel free to add "checkin-needed" keyword.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/UITest.java
(Diff revision 6)
>  
>  package org.mozilla.gecko.tests;
>  
>  import org.mozilla.gecko.Actions;
>  import org.mozilla.gecko.Assert;
> -import org.mozilla.gecko.BrowserApp;

nit: for future reference, changes such as these ideally should be in a Pre or Post commit; No need to change this now, just an FYI.
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 21

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/bc9db3800f3c
Settings title remains as "Language" after switching locale and returning to "General Settings". r=grisha
Keywords: checkin-needed

Comment 22

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bc9db3800f3c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Verified as fixed in build 50.0a1 (2016-06-15);
Device: Nexus 5 (Android 6.0.1).
Daniel, are you going to request to uplift or let it ride the train from 50? Thanks
Flags: needinfo?(dlim)
Assignee

Comment 26

3 years ago
Approval Request Comment
[Feature/regressing bug #]: 1263726
[User impact if declined]: Settings title will be incorrect after switching locale and returning to "General Settings"
[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa933381917d
[Risks and why]: Low risk, settings title will be affected, it's read-only and doesn't affect content
[String/UUID change made/needed]: None
Flags: needinfo?(dlim)
Attachment #8765046 - Flags: approval-mozilla-aurora?
Comment on attachment 8765046 [details] [diff] [review]
bug-1263726-fix.patch

Please uplift to aurora, let's verify it on aurora once it lands.
Attachment #8765046 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in build 49.0a2 (2016-07-17);
Device: LG G4 (Android 5.1).
You need to log in before you can comment on or make changes to this bug.