Closed Bug 981941 Opened 6 years ago Closed 6 years ago

don't show the password autocomplete popup for about:accounts


(Firefox :: Sync, defect, P2)




Firefox 30
Tracking Status
firefox29 + verified
firefox30 --- verified


(Reporter: ckarlof, Assigned: Gavin)




(1 file, 2 obsolete files)

Seeing the password manager dialog on about:accounts is weird and it prompts them to set up sync which is confusing.
Component: Firefox Sync: Backend → Sync
Product: Mozilla Services → Firefox
Summary: respect autocomplete=off on about:accounts → don't show the password autocomplete popup for about:accounts
Attached patch hack patch (obsolete) — Splinter Review
Bug 956906 kind of screwed our plan for handling this issue.

This is one simple but ugly (very specific) way of accomplishing it.
Attachment #8388936 - Flags: feedback?(dolske)
Attached patch slightly less hacky patch (obsolete) — Splinter Review
This is another approach I thought of that is slightly more generic and thus maybe more tolerable. The "also check topmost window" bit is kind of arbitrary and specific to the about:accounts case. It doesn't work as-is because _getPasswordOrigin isn't compatible with "about:accounts" URIs, but you get the general idea.
Attachment #8388937 - Flags: feedback?(dolske)
Priority: -- → P2
Comment on attachment 8388936 [details] [diff] [review]
hack patch

Review of attachment 8388936 [details] [diff] [review]:

::: toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ +419,5 @@
> +            if (topWin.document && topWin.document.documentURI == "about:accounts") {
> +                log("(form submission ignored -- about:accounts)");
> +                return;
> +            }
> +        }

I'd kill the pref. Shouldn't || always be true, since we're in a form-submit handler?

That should make the code shorter than your comment. Which could also be briefer. :)
Attachment #8388936 - Flags: feedback?(dolske) → feedback+
Comment on attachment 8388937 [details] [diff] [review]
slightly less hacky patch

Review of attachment 8388937 [details] [diff] [review]:

I'd just as soon keep the hack simple; if we find needing to do this on more pages that are comfortably handled in an "if(a||b)" then we can extensify it.
Attachment #8388937 - Flags: feedback?(dolske) → feedback-
Attached patch patchSplinter Review
Attachment #8388936 - Attachment is obsolete: true
Attachment #8388937 - Attachment is obsolete: true
Attachment #8390884 - Flags: review?(dolske)
Comment on attachment 8390884 [details] [diff] [review]

Review of attachment 8390884 [details] [diff] [review]:

::: toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ +406,5 @@
> +        // Somewhat gross hack - we don't want to show the "remember password"
> +        // notification on about:accounts for Firefox.
> +        let topWin =;
> +        if (/^about:accounts($|\?)/i.test(topWin.document.documentURI)) {

Attachment #8390884 - Flags: review?(dolske) → review+
Target Milestone: --- → Firefox 30
Attachment #8390884 - Flags: approval-mozilla-aurora+
Closed: 6 years ago
Resolution: --- → FIXED
Keywords: verifyme
QA Contact: twalker
I was able to reproduce the issue with Nightly build from March 10, on the about:accounts page, when signing in with existing account or signing up with new account.

The "remember password" notification no longer displays on about:accounts, either when trying to sign in or when trying to sign up. Verified with latest 30 Aurora (Build ID: 20140425004002) and 29 RC (Build ID: 20140421221237), on Win 7 x64, Mac OS X 10.7.5, Ubuntu 12.10 x86.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.