1185778 - Form suggestions should be disabled for the username field on edit logins page

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Logins, Passwords and Form Fill, defect)

Comment 1

[:Margaret Leibovic]

This should be as simple as setting autocomplete="off" on that input.

Comment 2

[Anirudh S [:anirudh24seven]]

Attachment: Form suggestions should be disabled for the username field on edit logins page

Comment 3

[Allison Naaktgeboren :ally]

Comment on attachment 8659389 [details] [diff] [review]
Form suggestions should be disabled for the username field on edit logins page

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

The patch itself looks good. Excellent start.

Now the tricksy part: making sure it doesn't break anything else. What STR did you use to test this? Have you run the tests for aboutLogins locally? Have you done a try run, https://wiki.mozilla.org/ReleaseEngineering/TryServer#How_to_push_to_try? 

It is normal for changes in behavior to result in failing tests, so please don't be discouraged. If you need help, I can push to try for you or help you with running tests locally. Please NEEDINFO me if you have questions or need help.

The team can be found on the IRC.mozilla.org, https://developer.mozilla.org/en-US/docs/Mozilla/QA/Getting_Started_with_IRC in the #mobile channel.

Comment 4

[Anirudh S [:anirudh24seven]]

I noticed that the following files under /mobile/android/tests/ are related to aboutLogins:

- /mobile/android/tests/browser/chrome/chrome.ini
- /mobile/android/tests/browser/chrome/test_about_logins.html
- /mobile/android/tests/browser/robocop/StringHelper.java 
- /mobile/android/tests/browser/robocop/testSystemPages.java 

I wanted to run Robocop tests locally and tried following the instructions provided here: https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#Robocop

I am unable to locate xpcshell though. Should I build Firefox for Desktop or something like that to obtain xpcshell? When I run mach robocop, I get this error: "environment variable MOZ_HOST_BIN must be set to a directory containing host xpcshell". Can I know how to proceed here?

Regarding pushing to try, I got acquainted with it when pushing a fix for this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1197874. I used this try chooser text earlier: try: -b do -p android-api-9,android-api-11 -u all -t none

What will be the try chooser text if I want to run Robocop and Mochitest chrome tests on the try server?

Comment 5

[Allison Naaktgeboren :ally]

(In reply to Anirudh S [:anirudh24seven] from comment #4)
> I noticed that the following files under /mobile/android/tests/ are related
> to aboutLogins:
> 
> - /mobile/android/tests/browser/chrome/chrome.ini
This is a build goop file

> - /mobile/android/tests/browser/chrome/test_about_logins.html
This is an actual test

> - /mobile/android/tests/browser/robocop/StringHelper.java 
This is, as the name suggests, a nice helper class for robocop tests

> - /mobile/android/tests/browser/robocop/testSystemPages.java 
This is a test driver class.

So the one you'll want to study is test_about_logins.html
> 
> I wanted to run Robocop tests locally and tried following the instructions
> provided here: https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#Robocop
> 
> I am unable to locate xpcshell though. Should I build Firefox for Desktop or
> something like that to obtain xpcshell? When I run mach robocop, I get this
> error: "environment variable MOZ_HOST_BIN must be set to a directory
> containing host xpcshell". Can I know how to proceed here?

You don't need to build desktop, but you do have to explictly set up the host for xpcshell
https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#Host_Builds_.28MOZ_HOST_BIN.29 has directions and an explanation

> 
> Regarding pushing to try, I got acquainted with it when pushing a fix for
> this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1197874. I used this
> try chooser text earlier: try: -b do -p android-api-9,android-api-11 -u all
> -t none
> 
> What will be the try chooser text if I want to run Robocop and Mochitest
> chrome tests on the try server?

Try syntax can get a wee bit painful. I use http://trychooser.pub.build.mozilla.org/ and check the boxes for which tests on which platforms I want to run. The run finishes faster and I use less resources. win win for everyone.

Comment 6

[Anirudh S [:anirudh24seven]]

Do I have to modify the test file to cover this change? The existing test doesn't seem to be targeting such a deep scope. 

I ran tests locally and also did a try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0056d9ae023 In the try run, a few tests seem to have timed out but I don't think it is related to my change. 

What will my next steps be?

Comment 7

[Allison Naaktgeboren :ally]

Usually, your next step would be to make a better case as to why those failures can't be related to your patch. However, in this case, your next step is pay careful attention and learn. :) 

It's tricksy, but please remember just because a test failure isn't obviously related to your patch, does not mean that 
a) your patch isn't making a race condition worse
b) violating some precondition elsewhere
c) poisoning another test 

Let's have a walk through of what I look for so you can make your case next time.
 
Let's have a look
Android 2.3 API9 opt 	
test suite 13 failed twice. uh-oh.

Android 2.3 API9 debug
no tests

Android 4.0 API11+ opt
"# TBPL FAILURE #"

Android 4.0 API11+ debug
no tests 	

Android 4.3 API11+ opt
test suite 13 failed twice

Android 4.3 API11+ debug
10 failed then succeeded on re-try
13 failed then succeeded on re-try

So the TBPL failure is a failure of the build system, which is not your fault. However it does mean that the state of tests 4.0 API11+ opt is unknown since they never ran.

13 fails on a number of platforms and consistently, so I'm suspicious of that. However I know there was a period where this test was failing on try for everyone, so I've kicked off a number of re-tries of the failed runs.

If they go green, we're good. If they don't I'll download your patch and kick off a try run on the latest build.

If _that_ fails, there is a nasty bug hiding out somewhere we'll need to squash before we can land your patch.

Comment 8

[Allison Naaktgeboren :ally]

the retriggered tests stayed orange. Time to re-try.

Comment 9

[Allison Naaktgeboren :ally]

minty fresh try https://treeherder.mozilla.org/#/jobs?repo=try&revision=54aa5d5ea78a

Comment 10

[Allison Naaktgeboren :ally]

OK, it looks like that test is just hosed. The tests stay orange on try even when I push an empty patch.

Comment 11

[Allison Naaktgeboren :ally]

Comment on attachment 8659389 [details] [diff] [review]
Form suggestions should be disabled for the username field on edit logins page

Congrats on getting through the gauntlet!

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6ce747756f92

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ce747756f92

Comment 14

[Teodora Vermesan (:TeoVermesan)]

Go to Tools -> Logins, tap on a login and choose "Edit login". Delete the username and tap the field. No form suggestions are displayed, so:
Verified as fixed using:
Device: Samsung Galaxy Note 5 (Android 5.1.1)
Build: Firefox for Android 44.0a1 (2015-10-04)