Closed Bug 1114821 Opened 5 years ago Closed 4 years ago

Remove nightly-only flag for about logins


(Firefox for Android :: General, defect)

Not set



Firefox 43
Tracking Status
firefox42 --- fixed
firefox43 --- fixed
fennec 42+ ---


(Reporter: mcomella, Assigned: ally)




(1 file, 2 obsolete files)

Assuming part 2 of bug 1095278 lands as is.
including testAboutPasswords
Summary: Remove nightly-only flag on testAboutPasswords → Remove nightly-only flag for about logins
Assignee: nobody → ally
Attached patch aboutLoginsRemoveNightlyFlags (obsolete) — Splinter Review
Attachment #8644062 - Flags: review?(liuche)
Comment on attachment 8644062 [details] [diff] [review]

Review of attachment 8644062 [details] [diff] [review]:

This looks mostly good, but you're missing changes in AboutRedirector.js, the menu item in,, and a few tests (testSystemPages, testSettingsMenuItems). This is just from a quick mxr search, so double check to make sure there aren't any other places where you're missing NIGHTLY flags.
Attachment #8644062 - Flags: review?(liuche) → review-
Let's make sure we uplift this after it's done.
tracking-fennec: --- → 42+
pending try results
Attachment #8644062 - Attachment is obsolete: true 
weekend's try busted. let's see if I got it all this time.
try's happy.
Attachment #8645801 - Attachment is obsolete: true
Attachment #8645908 - Flags: review?(liuche)
Chenxia, any estimate as to when you might get around to this review? Would you like me to find another reviewer?
Flags: needinfo?(liuche)
Comment on attachment 8645908 [details] [diff] [review]

Review of attachment 8645908 [details] [diff] [review]:

A little more cleanup, but this is good to go.

::: mobile/android/base/preferences/
@@ -735,5 @@
>                          preferences.removePreference(pref);
>                          i--;
>                          continue;
>                      }
> -                } else if (PREFS_LOGIN_MANAGE.equals(key)) {

Remove this local variable declaration too, as well as the android:key from the corresponding xml layout file.

::: mobile/android/tests/browser/robocop/
@@ -195,5 @@
>          }
> -        if (!AppConstants.NIGHTLY_BUILD) {
> -            final List<String[]> privacy = settingsMap.get(PATH_PRIVACY);
> -            privacy.remove(MANAGE_LOGINS_ARR);

Remove this array declaration since it's unused now.

::: mobile/android/tests/browser/robocop/
@@ -98,5 @@
>              verifyTabCount(expectedTabCount);
>          }
>      }
>      private boolean skipItemURL(String item) {

Remove this entire method, because this is the only used for logins and we'd just be making a bunch of calls to a method that always returns false.
Attachment #8645908 - Flags: review?(liuche) → review+
Flags: needinfo?(liuche)
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment on attachment 8645908 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: about:logins
[User impact if declined]: users will be unable to access to login/password manager at all
[Describe test coverage new/current, TreeHerder]:all logins tests have been unblocked and will no longer be skipped
[Risks and why]:  ^
[String/UUID change made/needed]: none
Attachment #8645908 - Flags: approval-mozilla-aurora?
Comment on attachment 8645908 [details] [diff] [review]

We want this feature to be available.
Attachment #8645908 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Release Note Request (optional, but appreciated)
[Why is this notable]: I guess this is a new feature on mobile
[Suggested wording]: Login and password manager
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
You need to log in before you can comment on or make changes to this bug.