Closed Bug 1149500 Opened 5 years ago Closed 9 months ago

Delay autofill on background tabs until selected

Categories

(Toolkit :: Password Manager, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: phlsa, Assigned: sfoster)

References

(Depends on 3 open bugs, Blocks 3 open bugs)

Details

(Whiteboard: [passwords:master-password] security:passwords)

Attachments

(1 file)

The timing when the (modal!) master password prompt appears is seemingly arbitrary. The user is prompted once per session, but sometimes that prompt happens directly on startup, sometimes a few minutes later and sometimes only when actually filling a password (even then the timing is bad since it appears before the page is even loaded).

We should find a way to make that behavior more predictable
Mistakenly filed against Firefox 38 and should be instead 38 Branch. Sorry for the spam. dkl
Version: Firefox 38 → 38 Branch
This is currently set for Platform Mac OS, but I am pretty sure it's the same under Windows. Change to all?
I'm going to morph this bug to cover the case where Sync is not setup.

We should either:
A) not autofill on page load and require the user to use the autocomplete dropdown or context menu
or
B) not autofill for background tabs, wait until the tab becomes focused. We can use the visibilitychange event for this.
Component: Security → Password Manager
OS: Mac OS X → All
Product: Firefox → Toolkit
Hardware: x86 → All
Summary: Master password prompt appears seemingly arbitrary → Don't show Master Password dialogs (or autofill) upon page load if a MP is set
Priority: -- → P2
Whiteboard: [ux-qx][qx] → [passwords:master-password]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #3)

We should either:
A) not autofill on page load and require the user to use the autocomplete
dropdown or context menu

This doesn't really work since the username is also encrypted so we wouldn't be able to show them without first getting the MP. If we wait to show the MP when autocomplete opens then the MP dialog will cause autocomplete to close which would be bad.

B) not autofill for background tabs, wait until the tab becomes focused. We
can use the visibilitychange event for this.

We should implement this for now as it helps delay the MP dialog at least until the login page is visible and prevents tracking the user via autofill in hidden frames or background tabs/popups.

Summary: Don't show Master Password dialogs (or autofill) upon page load if a MP is set → Don't autofill (or show a Master Password dialog) on background tabs
Whiteboard: [passwords:master-password] → [passwords:master-password] security:passwords
Version: 38 Branch → Trunk
Summary: Don't autofill (or show a Master Password dialog) on background tabs → Delay autofill (and the Master Password dialog) on background tabs until selected
Assignee: nobody → sfoster
Status: NEW → ASSIGNED

(In reply to Sam Foster [:sfoster] (he/him) from comment #7)

Created attachment 9044372 [details]
Bug 1149500 - Delay autofill until the document is visible. r?MattN

MattN: This is a first pass at this, with a couple of tests to verify the defer form autofill until the document is visible thing. I'm not sure if there are other cases that should be covered by tests - I see various secure/not, cross-origin, iframed cases in browser_http_autofill.js, but it wasnt clear to me if covering all those again here would be valuable.

Other cases which come to mind - closing a tab without ever making it visible, navigating away from the form in a hidden tab. Destroying a form while the document is hidden?

The case where logins are added while the document is hidden that might change how it is autofilled - should just work with this approach.

Oh and master password. I could add an explicit test with LoginTestUtils.masterPassword.enable().

(In reply to Sam Foster [:sfoster] (he/him) from comment #8)

(In reply to Sam Foster [:sfoster] (he/him) from comment #7)

Created attachment 9044372 [details]
Bug 1149500 - Delay autofill until the document is visible. r?MattN

MattN: This is a first pass at this, with a couple of tests to verify the defer form autofill until the document is visible thing. I'm not sure if there are other cases that should be covered by tests - I see various secure/not, cross-origin, iframed cases in browser_http_autofill.js, but it wasnt clear to me if covering all those again here would be valuable.

I don't think so. For some reason I thought visibilityState was on a per-frame basis but now I see that the top-level browsing context is all that matters so I guess you don't need to test with sub-frames.

Other cases which come to mind - closing a tab without ever making it visible, navigating away from the form in a hidden tab. Destroying a form while the document is hidden?

I'm not sure you need those since those should mostly be testing visibilitychange/VisibilityState and WeakMaps since I believe we want to do nothing in those cases.

The case where logins are added while the document is hidden that might change how it is autofilled - should just work with this approach.

Agreed.

(In reply to Sam Foster [:sfoster] (he/him) from comment #9)

Oh and master password. I could add an explicit test with LoginTestUtils.masterPassword.enable().

I think that would be good. Just a simple one where there is a MP and it's logged out and a background login form tab opens with a matching saved login.

Depends on: 1304001
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d16bf53b3585
Delay autofill until the document is visible. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Depends on: 1538460
Depends on: 1539091
Regressions: 1538460

Due to regressions we stopped delaying the autofill if a master password is set (bug 1538460) so we didn't address the issue of MP prompts from background tabs in this bug. I filed bug 1567574 for that.

Summary: Delay autofill (and the Master Password dialog) on background tabs until selected → Delay autofill on background tabs until selected
You need to log in before you can comment on or make changes to this bug.