1217156 - Add a preference to turn on/off insecure password warnings

Status: RESOLVED FIXED

(Firefox :: Security, defect, P1)

Description

[Tanvi Vyas[:tanvi]]

We should add a pref to enable/disable insecure password warnings in case we decide we need to hold it back from release, but want to keep it in the other channels (especially dev edition).

Comment 1

[Tanvi Vyas[:tanvi]]

Attachment: Bug1217156-10-29-15.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc48e884c8ac

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8680901 [details] [diff] [review]
Bug1217156-10-29-15.patch

Review of attachment 8680901 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, I'll push to try on aurora and fx-team to make sure we are good there

::: browser/base/content/browser.js
@@ +6929,5 @@
>  
> +  get _hasInsecureLoginForms() {
> +    // checks if the page has been flagged for an insecure login. Also checks
> +    // if the flag to degrade the UI is set to true
> +    return (LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser) && Services.prefs.getBoolPref("security.insecure_password.ui.enabled"));

Nit - don't need the extra parens, and please wrap this into two lines:

return LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser) &&
       Services.prefs.getBoolPref("security.insecure_password.ui.enabled");

Comment 3

[Brian Grinstead [:bgrins]]

fx-team: https://treeherder.mozilla.org/#/jobs?repo=try&revision=385fa1bd69f3
aurora: https://treeherder.mozilla.org/#/jobs?repo=try&revision=25933d00995e

Comment 4

[Tanvi Vyas[:tanvi]]

Attachment: Bug1217156-10-29-15B.patch

Fixed nits.  Carrying over r+ from bgrins.  Needs check in and uplift.

Comment 5

[Tanvi Vyas[:tanvi]]

(In reply to Tanvi Vyas [:tanvi] from comment #4)
> Needs check in and uplift.

But I'll wait for some try results first.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3189c9d88f13

Comment 7

[Tanvi Vyas[:tanvi]]

Comment on attachment 8680915 [details] [diff] [review]
Bug1217156-10-29-15B.patch

Approval Request Comment
[Feature/regressing bug #]: https://bugzilla.mozilla.org/show_bug.cgi?id=1179961
[User impact if declined]: Developers will see insecure password warnings on HTTP pages AND localhost pages, without learn more links.  We need to fix localhost and add learn more before we can expose this to developers
[Describe test coverage new/current, TreeHerder]: There is a test for the feature that is still enabled.  This bug just disables the feature for everything but nightly.
[Risks and why]: Developers will become habituated to the warning since they will see it on localhost.  Which is the opposite of what we want - we want them to fix it.
[String/UUID change made/needed]: None

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3189c9d88f13

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8680915 [details] [diff] [review]
Bug1217156-10-29-15B.patch

Make sense, taking it.
Should be in the first devedition release.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/904dd138cbc9

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/3189c9d88f13

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/904dd138cbc9

Comment 13

[Tony Mechelynck [:tonymec]]

(In reply to Carsten Book [:Tomcat] from comment #10)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/904dd138cbc9
>        status-firefox44: affected → fixed

(In reply to Carsten Book [:Tomcat] from comment #12)
> https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/904dd138cbc9
>        status-b2g-v2.5: fixed → ---

Huh? This is the exact same patch. How can it fix the problem on desktop and unfix it on B2G?

Comment 14

[Carsten Book [:Tomcat]]

(In reply to Tony Mechelynck [:tonymec] from comment #13)
> (In reply to Carsten Book [:Tomcat] from comment #10)
> > https://hg.mozilla.org/releases/mozilla-aurora/rev/904dd138cbc9
> >        status-firefox44: affected → fixed
> 
> (In reply to Carsten Book [:Tomcat] from comment #12)
> > https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/904dd138cbc9
> >        status-b2g-v2.5: fixed → ---
> 
> Huh? This is the exact same patch. How can it fix the problem on desktop and
> unfix it on B2G?

it was a simple problem with the setup of the 2.5 branch. we merged mozilla-central to 2.5 and reverted this. (ended up doing a merge from mozilla-aurora instead of m-c).