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 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");

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

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

(In reply to Tanvi Vyas [:tanvi] from comment #4) > Needs check in and uplift. But I'll wait for some try results first.

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 on attachment 8680915 [details] [diff] [review] Bug1217156-10-29-15B.patch Make sense, taking it. Should be in the first devedition release.

(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?

(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).