Closed Bug 886916 Opened 11 years ago Closed 11 years ago

Regression: Multiple-level settings screens do not handle "back" correctly

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox23 verified, firefox24 verified, firefox25 verified)

VERIFIED FIXED
Firefox 25
Tracking Status
firefox23 --- verified
firefox24 --- verified
firefox25 --- verified

People

(Reporter: liuche, Assigned: liuche)

Details

Attachments

(4 files, 2 obsolete files)

In adding link handling for bug 879558, I broke back behavior for nested settings. Attaching a patch to create a new result code and not overload use of Activity.RESULT_CANCELED.

inbound try build: https://tbpl.mozilla.org/?tree=Try&rev=389c706b3327
Assignee: nobody → liuche
Status: NEW → ASSIGNED
Attachment #767343 - Flags: review?(rnewman)
Aurora try build: https://tbpl.mozilla.org/?tree=Try&rev=7e17d66a8cc8
Attachment #767345 - Flags: review?(mark.finkle)
Attachment #767348 - Flags: review?(mark.finkle)
Attachment #767343 - Flags: review?(rnewman) → review?(mark.finkle)
Attachment #767343 - Flags: review?(mark.finkle) → review?(margaret.leibovic)
Attachment #767345 - Flags: review?(mark.finkle) → review?(margaret.leibovic)
Attachment #767348 - Flags: review?(mark.finkle) → review?(margaret.leibovic)
Comment on attachment 767345 [details] [diff] [review]
Aurora patch: add new result code

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879558
User impact if declined: users manually navigating through Settings > Data Choices will be returned to browser screen on hitting back, instead of one level up the menu
Testing completed (on m-c, etc.): manual confirm of regression and fix of m-a
Risk to taking this patch (and alternatives if risky): very low, replaces a constant in an existing code path
String or IDL/UUID changes made by this patch: none
Attachment #767345 - Flags: approval-mozilla-aurora?
Comment on attachment 767348 [details] [diff] [review]
Beta patch: Add new result code for GeckoPreferences

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879558
User impact if declined: users manually navigating through Settings > Data Choices will be returned to browser screen on hitting back, instead of one level up the menu
Testing completed (on m-c, etc.): manual confirm of regression and fix of m-b
Risk to taking this patch (and alternatives if risky): very low, replaces a constant in an existing code path
String or IDL/UUID changes made by this patch: none
Attachment #767348 - Flags: approval-mozilla-beta?
Comment on attachment 767343 [details] [diff] [review]
Inbound patch: add new result

Review of attachment 767343 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoPreferences.java
@@ +75,5 @@
>      private static String PREFS_GEO_REPORTING = "app.geo.reportdata";
>      private static String PREFS_HEALTHREPORT_LINK = NON_PREF_PREFIX + "healthreport.link";
>  
>      private static int REQUEST_CODE_PREF_SCREEN = 5;
> +    private static int RESULT_CODE_EXIT_SETTINGS = 6;

Nit: Add a comment about why you chose these values.

@@ +208,5 @@
>  
>      @Override
>      public void onActivityResult(int requestCode, int resultCode, Intent data) {
> +        if (REQUEST_CODE_PREF_SCREEN == requestCode && RESULT_CODE_EXIT_SETTINGS == resultCode) {
> +            setResult(RESULT_CODE_EXIT_SETTINGS);

Nit: I know this is already in the current code, but it would be nice to add a comment here about why we're setting the result with what already appears to be the resultCode.
Attachment #767343 - Flags: review?(margaret.leibovic) → review+
Attachment #767345 - Flags: review?(margaret.leibovic) → review+
Attachment #767348 - Flags: review?(margaret.leibovic) → review+
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879558
User impact if declined: users manually navigating through Settings > Data Choices will be returned to browser screen on hitting back, instead of one level up the menu
Testing completed (on m-c, etc.): manual confirm of regression and fix of m-b
Risk to taking this patch (and alternatives if risky): very low, replaces a constant in an existing code path
String or IDL/UUID changes made by this patch: none
Attachment #767348 - Attachment is obsolete: true
Attachment #767348 - Flags: approval-mozilla-beta?
Attachment #767424 - Flags: approval-mozilla-beta?
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879558
User impact if declined: users manually navigating through Settings > Data Choices will be returned to browser screen on hitting back, instead of one level up the menu
Testing completed (on m-c, etc.): manual confirm of regression and fix of m-a
Risk to taking this patch (and alternatives if risky): very low, replaces a constant in an existing code path
String or IDL/UUID changes made by this patch: none
Attachment #767345 - Attachment is obsolete: true
Attachment #767345 - Flags: approval-mozilla-aurora?
Attachment #767427 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/05ba2e9812df
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Attachment #767427 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #767424 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: RESOLVED → VERIFIED
Verified as fixed in build: Firefox 23;
Device: Asus Transformer Tab (Android 4.0.3).
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: