413768 - Clicking in almost any <input type="text"> when the Keychain is locked will prompt

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

STR:

1. Set your Keychain to auto-lock after so many minutes, or lock it manually
2. With the Keychain locked up, wisit some site that has an single-line text input field, e.g. the search field at the wiki.
3. Place your cursor in said field.

AR: Prompted to allow Camino to access (unlock) your Keychain.

ER: Just a blinking cursor, ma'am.

This is one of the more annoying regressions from multiple accounts.

Comment 1

[Bryan Atwood]

Attachment: patch

Ugh.  This was one of Mento's suggestions that I didn't implement correct.  Function changed so that it returns Error message when there is no password field found.

Can you file bugs on other regressions?

Comment 2

[Mark Mentovai]

Comment on attachment 298843 [details] [diff] [review]
patch

At end of FindUsernamePasswordFields:

>-  return NS_OK;
>+  return NS_ERROR_FAILURE;

Shouldn't that one stay NS_OK?

In FindPasswordField:

>-  if (NS_FAILED(rv) || usernameElement != inUsername)

>+  if (NS_FAILED(rv) || usernameElement != inUsername || !*outPassword)

Do you really need to add that?  The changes to FindUsernamePasswordFields should ensure that NS_FAILED(rv) will be true any time *outPassword is false, right?

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #1)

> Can you file bugs on other regressions?

Er, "regressions" wasn't exactly the word I meant, sorry; "regression among the follow-ups" or something of that sort.  This is the only real regression I've seen so far, other than the sort-of-regression bug 413400.

Comment 4

[Bryan Atwood]

Attachment: Patch for comment #2

(In reply to comment #2)
> (From update of attachment 298843 [details] [diff] [review])
> At end of FindUsernamePasswordFields:
> 
> >-  return NS_OK;
> >+  return NS_ERROR_FAILURE;
> 
> Shouldn't that one stay NS_OK?

It's actually:

  return (*outPassword && *outUsername) ? NS_OK : NS_ERROR_FAILURE;

since we could get here at the end of the form where the last element is either a password or not and inStopWhenFound is false.

> 
> In FindPasswordField:
> 
> >-  if (NS_FAILED(rv) || usernameElement != inUsername)
> 
> >+  if (NS_FAILED(rv) || usernameElement != inUsername || !*outPassword)
> 
> Do you really need to add that?  The changes to FindUsernamePasswordFields
> should ensure that NS_FAILED(rv) will be true any time *outPassword is false,
> right?
> 

OK, I was overly paranoid.  Still bummed that I missed this error.  I removed the additional check for *outPassword.

Comment 5

[Mark Mentovai]

Comment on attachment 298876 [details] [diff] [review]
Patch for comment #2

>+  return (*outPassword && *outUsername) ? NS_OK : NS_ERROR_FAILURE;

Of course!

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Checked in on the trunk and the MOZILLA_1_8_BRANCH well in advance of me losing my sanity (er, of b3) ;)

Thanks, Bryan!