Closed Bug 408363 Opened 14 years ago Closed 14 years ago

Password Manager remember prompt gets dismissed too early

Categories

(Toolkit :: Password Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

()

Details

Attachments

(2 files)

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1

Steps to reproduce:

1. Go to <http://www.editorialmanager.com/dapd/>.
2. Select Login from the top menu.
3. Enter any username and password, and hit Author Login.  The remember password bar appears.
4. Don't do anything, and wait to be redirected to <http://www.editorialmanager.com/dapd/default.asp?pg=login.asp%3FloginError%3D1>.

As soon as the redirection occurs, the remember password bar disappears, without allowing the user to see if her login was successful or not before attempting to save the password, thereby voiding the rationale behind the new bar.

Expected result: the bar should not be dismissed upon page redirect.
Flags: blocking-firefox3?
The behavior that we settled on in bug 226735 was to ignore the first location change after showing the notification bar, and all other locations until 10 seconds after the sign in. Does your test site wait more than 10 seconds to redirect you? Is that normal behavior? Not sure we want to fix this.
(In reply to comment #1)
> The behavior that we settled on in bug 226735 was to ignore the first location
> change after showing the notification bar, and all other locations until 10
> seconds after the sign in. Does your test site wait more than 10 seconds to
> redirect you? Is that normal behavior? Not sure we want to fix this.
> 

Well, 10 seconds is too little on slow connections...  We intend to make the browser usable both on broad-band and dial-up, right?

What is wrong with just letting the notification bar sit there for the whole period of the page being loaded, and remove it 10 seconds after the page has finished loading, or for example when a connection timeout error occurs?
I looked at handling redirects in bug 226735 comment 62, and it was looking very ugly. The timeout isn't perfect, but it was simple.(In reply to comment #2)
Not blocking on the assumption that this is the 10s timeout thing as per comment 1 and comment 3.

Ehsan: are you sure that it's the 10s thing that's causing this? Can you always reproduce as you say in comment 0 where the bar disappears at the exact same time as the redirect? If so, please re-nom.
Flags: blocking-firefox3? → blocking-firefox3-
(In reply to comment #4)
> Not blocking on the assumption that this is the 10s timeout thing as per
> comment 1 and comment 3.
> 
> Ehsan: are you sure that it's the 10s thing that's causing this? Can you always
> reproduce as you say in comment 0 where the bar disappears at the exact same
> time as the redirect? If so, please re-nom.

I'm currently on a fast connection, but my further testing shows that this behavior is caused by the 10 seconds timeout, which doesn't look too good on dial-up connections.  I see this problem all the time on dial-up.  Other logins, such as gmail's, suffers from the same problem as well.

Could the 10-second timeout at least be increased?
Summary: Password Manager remember prompt gets dismissed automatically upon a redirect → Password Manager remember prompt gets dismissed too early
What's the harm in just leaving the notification bar open during all the redirects?  That's what happens if the redirects finish sooner than 10 seconds and the page loads: the notification bar just sits there waiting for an action from the user.  Isn't it?
The "did the location change?" code doesn't know why the location changed (HTTP redirect, user click, typed URL, javascript, etc). So if we don't dismiss it at some location change, it would persist through your entire browsing session (unless you manually close it, which is annoying).

The 10 second value is fairly arbitrary. Would 15 or 20 be better? Not sure if I'd want to go above that.
(In reply to comment #7)
> The "did the location change?" code doesn't know why the location changed (HTTP
> redirect, user click, typed URL, javascript, etc). So if we don't dismiss it at
> some location change, it would persist through your entire browsing session
> (unless you manually close it, which is annoying).

That's too unfortunate, so there *has* to be a timeout, right?

By the way, is it possible to change the code responsible for handling location changes in a way to make it more smart about the source of the location change events?  Is there a bug filed on that?

> The 10 second value is fairly arbitrary. Would 15 or 20 be better? Not sure if
> I'd want to go above that.

I think 20 seconds is a better choice, since it's close to the patience of many dial-up users for a page to load before getting tired and canceling the process.  Based on my experience, that is.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #293636 - Flags: review?(dolske)
Target Milestone: --- → Firefox 3 M11
Comment on attachment 293636 [details] [diff] [review]
Bump the timeout value from 10 to 20 seconds

I can live with this. I don't think the comment about why the timeout is bumped is needed, though.
Attachment #293636 - Flags: review?(gavin.sharp)
Attachment #293636 - Flags: review?(dolske)
Attachment #293636 - Flags: review+
Comment on attachment 293636 [details] [diff] [review]
Bump the timeout value from 10 to 20 seconds

Agreed with dolske on the comment being unnecessary.
Attachment #293636 - Flags: ui-review?(beltzner)
Attachment #293636 - Flags: review?(gavin.sharp)
Attachment #293636 - Flags: review+
Beltzner: waiting for your call on this.
Comment on attachment 293636 [details] [diff] [review]
Bump the timeout value from 10 to 20 seconds

uir+a=beltzner
Attachment #293636 - Flags: ui-review?(beltzner)
Attachment #293636 - Flags: ui-review+
Attachment #293636 - Flags: approval1.9+
This patch is the same as attachment 293636 [details] [diff] [review], with the comment removed as per comments 9 and 10.
Requesting check-in of attachment 299960 [details] [diff] [review]...
Keywords: checkin-needed
Checking in toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js,v  <--  nsLoginManagerPrompter.js
new revision: 1.18; previous revision: 1.17
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
OS: Windows Vista → All
Hardware: PC → All
Resolution: --- → FIXED
Deb's commented that this makes it somewhat hard to predict when the notification will go away vs. when it won't. We might want to change the logic to be more based on user input (ie: when the user clicks vs. automatically refreshing) or crank down the delay a little.

That'd be a new bug, though. :)
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.