Closed Bug 1446199 Opened 2 years ago Closed Last year

Move image-animation and media-encryption settings to new Media prefpane

Categories

(SeaMonkey :: Preferences, defect)

SeaMonkey 2.57 Branch
defect
Not set

Tracking

(seamonkey2.49esr unaffected, seamonkey2.53 affected, seamonkey2.57esr fixed)

RESOLVED FIXED
seamonkey2.58
Tracking Status
seamonkey2.49esr --- unaffected
seamonkey2.53 --- affected
seamonkey2.57esr --- fixed

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

Details

(Whiteboard: [late-l10n implications for Access Keys and Help])

Attachments

(3 files, 1 obsolete file)

14.07 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: ui-review+
Details | Diff | Splinter Review
4.40 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-beta+
Details | Diff | Splinter Review
3.73 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-beta+
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #493217 +++

(Quoting :frg from bug 493217 comment #22)
> The new media pane is almost empty now but we should fill it up with
> image and other stuff later.

Opening this follow-up bug to move the "Animated images should loop" groupbox and radiogroup from Privacy & Security > Images to Appearance > Media. I'd think that "Image Acceptance Policy" should stay in Privacy, given that it's indeed a privacy feature (tracking pics, etc.).

The only other option which I could find that should be moved is the Encrypted Media Extension checkbox, currently in Advanced > Scripts & Plugins. This is a don't-care for 2.57esr as it's not available yet and omitted for beta and release builds anyway.

I'm taking this since those are fairly straight-forward moves and I've worked on the EME box already before, unless Bill had planned on continuing to work on the Media pane after bug 493217, in which case he should yell at you or me and I'll yield.  ;-)
Attached patch Patch for trunkSplinter Review
Here we go, Help content comes in a separate patch in case it needs more discussion.

I've kept the groupboxes as they are. First I thought to add the EME checkbox simply into the Audio/Video group, but given that it's coming with long labels and looked awkward as in

  +- Audio/Video ----------------------------------+
  |  [ ] Enable Autoplay for HTML5 media content   |
  |  Enable Digital Right Management for           |
  |    [ ] Third-party Content Decryption Modules  |
  +------------------------------------------------+

I've kept the two separate boxes (which also may become handy if more options of either group are added later, thus not intermixing these two functions).
Attachment #8961846 - Flags: ui-review?(iann_bugzilla)
Attachment #8961846 - Flags: review?(frgrahl)
Attached patch Patch for betaSplinter Review
Much simpler than the trunk version as the EME checkbox is hidden anyway.
Attachment #8961847 - Flags: review?(frgrahl)
Attached patch Help patch (for both) (obsolete) — Splinter Review
I have expanded on the Autoplay option a bit as it was rather short, also mentioned that this is HTML5 only and not effective for Plugins playing media.

The image-animation content was straight carried over, no changes.
Attachment #8961849 - Flags: review?(iann_bugzilla)
BTW: I've also made some white-space changes for alignment for the Autoplay option, so these are not actual code modifications in the interdiffs.
Attached patch Help patch (v2)Splinter Review
I have noticed that the styles of how to represent groupboxes in Help is different between the Privacy/Security and Appearance prefpanes. For the latter, rather than using <h3> sub-grouping, nested <ul> are used. Thus, I have adapted the changes to match that style (also expanded the "not for plugin" not to animated images).
Attachment #8961849 - Attachment is obsolete: true
Attachment #8961849 - Flags: review?(iann_bugzilla)
Attachment #8961856 - Flags: review?(iann_bugzilla)
Comment on attachment 8961846 [details] [diff] [review]
Patch for trunk

Just wondering if image.animation_mode should come first or second rather than last, but seems fine apart from that.
Flags: needinfo?(rsx11m.pub)
Attachment #8961846 - Flags: ui-review?(iann_bugzilla) → ui-review+
Comment on attachment 8961856 [details] [diff] [review]
Help patch (v2)

r=me LGTM
Attachment #8961856 - Flags: review?(iann_bugzilla) → review+
(In reply to Ian Neal from comment #6)
> Just wondering if image.animation_mode should come first or second rather than last, but seems fine
> apart from that.

When I head "Media" I think primarily of Videos first, then Audio and then Images, thus I think the order should be ok.
Flags: needinfo?(rsx11m.pub)
Comment on attachment 8961847 [details] [diff] [review]
Patch for beta

[Approval Request Comment]
Regression caused by (bug #): follow-up feature
User impact if declined: animation settings will remain at old location
Testing completed (on m-c, etc.): c-c, applies to c-b
Risk to taking this patch (and alternatives if risky): none
String changes made by this patch: none, but potential for accesskey conflicts
Attachment #8961847 - Flags: approval-comm-beta?
Comment on attachment 8961856 [details] [diff] [review]
Help patch (v2)

[Approval Request Comment]
Help changes only
Attachment #8961856 - Flags: approval-comm-beta?
Comment on attachment 8961846 [details] [diff] [review]
Patch for trunk

LGTM r=me
Attachment #8961846 - Flags: review?(frgrahl) → review+
Comment on attachment 8961847 [details] [diff] [review]
Patch for beta

r/a=me LGTM
Attachment #8961847 - Flags: review?(frgrahl)
Attachment #8961847 - Flags: review+
Attachment #8961847 - Flags: approval-comm-beta?
Attachment #8961847 - Flags: approval-comm-beta+
Comment on attachment 8961856 [details] [diff] [review]
Help patch (v2)

a=me
Attachment #8961856 - Flags: approval-comm-beta? → approval-comm-beta+
Pushed by rsx11m.pub@gmail.com:
https://hg.mozilla.org/comm-central/rev/b3b8f206b26f
Move image-animation and media-encryption settings to new Media prefpane. ui-r,r=IanN
https://hg.mozilla.org/comm-central/rev/26af92906dd9
Help updates for image-animation settings, also expanding Autoplay section. r=IanN
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Seamonkey2.58
Duplicate of this bug: 90837
Message posted to l10n mailing list for changes on comm-beta:
https://groups.google.com/forum/#!topic/mozilla.dev.l10n/4dG2Wq-8DYk
You need to log in before you can comment on or make changes to this bug.