733754 - Deactivate the 'OK' button if there is no text entered in Master Password credentials textbox

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul Feher]

Build: Nightly:Fennec/13.0a1 (2012-03-06)
Device: HTC Desire Z 
OS: Android 2.3.3

Steps to reproduce:
1. Open Fennec App
2. Tap on device Menu button > More > Settings
3. Tap on Use Master Password option and add one
4. Tap on Use Master Password option again

Expected Result:
The OK button should be inactive if the text field is empty.

Actual Result: 
The OK button is active.

Comment 1

[Aaron Train [:aaronmt]]

Does this really matter? If it's blank, you'll get a toast notification saying "Incorrect Password".

Comment 2

[Tony Chung [:tchung]]

Attachment: screenshot of active OK button

note from screenshot, there is no text in the password box, but the OK button is active.   From the bug, this button should be inactive

Comment 4

[Michael Kohler [:mkohler]]

Attachment: Patch v1

I couldn't do the same for the "Enter Master Password" dialog which appears after clicking the "Remember" password after you login to a site (http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/LoginManagerPrompter.js#216). Where does this code live? I'd be glad to fix that too.

(In reply to Aaron Train [:aaronmt] from comment #1)
> Does this really matter? If it's blank, you'll get a toast notification
> saying "Incorrect Password".

Actually, this is not the case. The window just appears again. Should we fix this? (Should also be fixed in the above mentioned dialog.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 661511 [details] [diff] [review]
Patch v1

Brian is a good reviewer for this

Comment 6

[Wesley Johnston (:wesj)]

(In reply to Michael Kohler [:michaelkohler] from comment #4)
> I couldn't do the same for the "Enter Master Password" dialog which appears
> after clicking the "Remember" password after you login to a site
> (http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/
> LoginManagerPrompter.js#216). Where does this code live? I'd be glad to fix
> that too.

That will require basically rewriting the prompt service, or rewriting loginManagerPrompter to send its own messages to Java and then writing up a more advanced bridge between the java prompt service and gecko.... Or we could force any prompts showing a password to disable the "positive" button if the password is blank, but I think there are probably valid use cases for allowing blank passwords. I don't think I'd accept the simple solution (special prompts for the LoginManager). I'd like to remove more of the special native UI code than add more.

i.e. its non-trivial in any case.

Comment 7

[Brian Nicholson (:bnicholson)]

Comment on attachment 661511 [details] [diff] [review]
Patch v1

+        EditText input1 = null;
...
+            String text1 = input1.getText().toString();

nit: I'd remove the 1 after the variable names.

+                                input.requestFocus();

What is this doing? I don't see any difference if I remove this line.

Comment 8

[Brian Nicholson (:bnicholson)]

Comment on attachment 661511 [details] [diff] [review]
Patch v1

r- until comments are answered

Comment 9

[Michael Kohler [:mkohler]]

Attachment: Patch v2 (review addressed)

You're absolutely right.

Comment 10

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70e1fa924492

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/70e1fa924492

Comment 12

[Cristian Nicolae (:xti)]

OK button is deactivated on the latest Nightly for the Master Password popup. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-12-06)
Device: Galaxy Tab2 7"
OS: Android 4.0.3