974742 - rename "signon.overrideAutocomplete" pref to be less ambiguous and update documentation

Status: RESOLVED DUPLICATE of bug 956906

(Toolkit :: Password Manager, defect)

Description

[Dave Garrett]

It's ambiguous what's being overridden by the pref name "signon.overrideAutocomplete". Setting this to true overrides a page's ability to override the password manager's autocompletion of passwords using 'autocomplete="off"'. Bug 956906 will change the default value to true. This pref should be inverted in meaning and renamed to "signon.allowSitesToDisablePasswordManager" or similar.

Comment 1

[Manish Goregaokar [:manishearth]]

Maybe we should change the pref name *first*, since bug 956906 involves changes to the documentation as well.

Comment 2

[Dave Garrett]

Well, that's why I suggested it in bug 956906 first. ;)

Comment 3

[Manish Goregaokar [:manishearth]]

Attachment: Rename to signon.allowSitesToDisablePasswordManager, switch polarity

I set it as signon.allowSitesToDisablePasswordManager, but we probably can get a better name for it.

Comment 4

[Dave Garrett]

Honestly, is there even a real need to let users change this back? Isn't there some way to selectively tell the password manager to never remember passwords for one site? I'd rather the pref were just ditched in favor of never applying the autocomplete flag to the password manager.

Comment 5

[Manish Goregaokar [:manishearth]]

There are security concerns, as listed in bug 956906. I don't think they're ultimately going to be a big issue, though.

People running Fx on shared systems may want to use it to provide their users with some additional security.

But yes, one possible way out is to just remove the functionality entirely. I guess adding the pref and setting it to false is just less drastic a change.

Comment 6

[Manish Goregaokar [:manishearth]]

Also, selectively telling the password manager to never remember passwords on a site does exist already.

But this is more of "if a site wants to disable passwords, let it" pref, which makes the change globally instead of having to enter the URL of every banking site under the sun in the exceptions list.

Comment 7

[Ben Bucksch (:BenB)]

+1 on a clearly named background (non-UI) pref that allows to keep the old behaviour

Comment 8

[Ben Bucksch (:BenB)]

Technical aspect in favor of renaming at same time as switching the default: when Firefox sees that the user value matches the default, it will not store the user value in prefs.js. That means that if you manually change the pref using about:config, then switch back and forth once between old and new builds, the user pref is deleted.
Workarounds:
* use user.js
* rename pref

Password storing is a hot topic that many many companies care about, so this is a good place to spend the little extra time to rename it to make the transition nicer.

Comment 9

[Manish Goregaokar [:manishearth]]

Comment on attachment 8378762 [details] [diff] [review]
Rename to signon.allowSitesToDisablePasswordManager, switch polarity

Changing reviewer to :adw since he seems to be handling the autocomplete business, and this bug blocks the others.

Comment 10

[Drew Willcoxon :adw]

Comment on attachment 8378762 [details] [diff] [review]
Rename to signon.allowSitesToDisablePasswordManager, switch polarity

Sorry for not giving feedback earlier.  I'm clearing the review request since the patch in bug 956906 covers this.  If that bug does end up covering this, we can close this bug.  (We don't usually track MDN documentation changes in bugs.)

Comment 11

[Dave Garrett]

Renamed in bug 956906. Pref is now "signon.storeWhenAutocompleteOff" (polarity not flipped).