Closed Bug 860970 Opened 7 years ago Closed 7 years ago

Need to sync new security prefs from bug 842191

Categories

(SeaMonkey :: Security, defect)

defect
Not set

Tracking

(seamonkey2.18 unaffected, seamonkey2.19 fixed, seamonkey2.20 fixed)

RESOLVED FIXED
seamonkey2.20
Tracking Status
seamonkey2.18 --- unaffected
seamonkey2.19 --- fixed
seamonkey2.20 --- fixed

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 842191 added four preferences to pref-ssl.xul but didn't sync them.
Attached patch Proposed patch (obsolete) — Splinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #736541 - Flags: review?(jh)
Attachment #736541 - Flags: review?(iann_bugzilla)
Neil don't you think we should also do this: http://hg.mozilla.org/mozilla-central/rev/60b08f643863

> 1.12 +// Block insecure active content on https pages
> 1.13 +pref("security.mixed_content.block_active_content", true);
Comment on attachment 736541 [details] [diff] [review]
Proposed patch

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

Please sort the prefs.

::: suite/browser/browser-prefs.js
@@ +930,5 @@
>  pref("services.sync.prefs.sync.security.warn_submit_insecure", true);
>  pref("services.sync.prefs.sync.security.warn_viewing_mixed", true);
> +pref("services.sync.prefs.sync.security.warn_mixed_active_content", true);
> +pref("services.sync.prefs.sync.security.mixed_content.block_active_content", true);
> +pref("services.sync.prefs.sync.security.warn_mixed_display_content", false);

What's the point of adding a pref that should not be synced?
(In reply to Philip Chee from comment #2)
> > 1.12 +// Block insecure active content on https pages
> > 1.13 +pref("security.mixed_content.block_active_content", true);
Sure, but in a separate bug.

(In reply to Jens Hatlak from comment #3)
> (From update of attachment 736541 [details] [diff] [review])
> Please sort the prefs.
Sorry, I hadn't noticed.

> > +pref("services.sync.prefs.sync.security.warn_mixed_display_content", false);
> What's the point of adding a pref that should not be synced?
Oops, too much copy & paste.
Attachment #736541 - Attachment is obsolete: true
Attachment #736541 - Flags: review?(jh)
Attachment #736541 - Flags: review?(iann_bugzilla)
Attachment #736697 - Flags: review?(jh)
Attachment #736697 - Flags: review?(jh) → review+
Pushed comm-central changeset aadbe43b0aa3.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.20
Comment on attachment 736697 [details] [diff] [review]
Addressed review comments

[Approval Request Comment]
Regression caused by (bug #): (New preferences - 842191)
User impact if declined: Some SSL preferences don't sync
Testing completed (on m-c, etc.): Landed on c-c
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None
Attachment #736697 - Flags: approval-comm-aurora?
One of these days I'll get these flags right first time...
Attachment #736697 - Flags: approval-comm-aurora? → approval-comm-aurora+
(In reply to neil@parkwaycc.co.uk from comment #8)
> One of these days I'll get these flags right first time...

Actually you don't need to set flags for the version that matches the TM. ;-)
You need to log in before you can comment on or make changes to this bug.