bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

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

RESOLVED FIXED

Status

Camino Graveyard
OS Integration
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Bryan Atwood)

Tracking

({fixed1.8.1.12})

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

1.40 KB, patch
Stuart Morgan
: review+
Mark Mentovai
: superreview+
Details | Diff | Splinter Review
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.
Flags: camino1.6b3?
Flags: camino1.6?
(Assignee)

Comment 1

11 years ago
Created attachment 298843 [details] [diff] [review]
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?
Assignee: nobody → batwood.bugs
Status: NEW → ASSIGNED
Attachment #298843 - Flags: review?(stuart.morgan)

Comment 2

11 years ago
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?
(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.
(Assignee)

Comment 4

11 years ago
Created attachment 298876 [details] [diff] [review]
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.
Attachment #298843 - Attachment is obsolete: true
Attachment #298876 - Flags: review?(stuart.morgan)
Attachment #298843 - Flags: review?(stuart.morgan)

Updated

11 years ago
Attachment #298876 - Flags: superreview?(mark)
Attachment #298876 - Flags: review?(stuart.morgan)
Attachment #298876 - Flags: review+

Comment 5

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

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

Of course!
Attachment #298876 - Flags: superreview?(mark) → superreview+
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
Checked in on the trunk and the MOZILLA_1_8_BRANCH well in advance of me losing my sanity (er, of b3) ;)

Thanks, Bryan!
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: camino1.6b3?
Flags: camino1.6?
Keywords: checkin-needed → fixed1.8.1.12
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.