Closed Bug 1127784 Opened 9 years ago Closed 9 years ago

[EME] Add a preference and UI to enable/disable playback of Encrypted Media Extensions

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(seamonkey2.40+ fixed)

RESOLVED FIXED
seamonkey2.40
Tracking Status
seamonkey2.40 + fixed

People

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

References

(Blocks 1 open bug, )

Details

Attachments

(4 files, 3 obsolete files)

(In Bug 1111146 comment #0)
> See user story in bug 1089876.
> 
> We want a master pref in Preferences -> Content to allow disabling all
> EME/CDM/DRM/BBQ support. It should be enabled by default, but allow a user
> to opt-out of DRM support.
> 
> Basically, a checkbox and link:
> 
>   [X] Play DRM Content   _Learn More_
> 
> Bug 1089876 comment 1 indicates this should basically be checkbox UI for the
> media.eme.enabled pref.
> 
> In support of the user stories in bug 1089873 and bug 1089882, the "Learn
> More" link provides an opportunity for the user to learn about what DRM is.
> (SUMO page TBD, initial wordsmithing in bug 1095734.)
....
> I believe we also want to use this pref to be used by users who are
> fundamentally opposed to DRM, so that any already-installed CDMs will be
> removed from their system. The exact way that functions is TBD -- we could
> immediately uninstall when clearing the checkbox, but it might be less of a
> footgun to uninstall at the next browser restart. [Again, depending on the
> order of this bug and the Addon Manager work, this part could be spun out
> into a followup.]

We also need to add the "media.eme.enabled" pref to browser-prefs.js
http://mxr.mozilla.org/comm-central/source/mozilla/browser/app/profile/firefox.js?rev=4898e64585e9&mark=1765-1767#1763
And possibly "browser.eme.ui.enabled" from:
https://hg.mozilla.org/mozilla-central/rev/4898e64585e9#l1.11
Err, so browser.eme.ui.enabled=false is supposed to hide the checkbox for media.eme.enabled?

Enabling EME by default (ok, it's disabled for release builds in Firefox) but then hiding the checkbox to switch it off doesn't appear to be the right thing to do. We could hide the checkbox in releases (similar to the geolocation pref) and leave it off by default until the feature is considered mature enough for the general user.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
(In reply to rsx11m from comment #1)
> Err, so browser.eme.ui.enabled=false is supposed to hide the checkbox for
> media.eme.enabled?
> 
> Enabling EME by default (ok, it's disabled for release builds in Firefox)
> but then hiding the checkbox to switch it off doesn't appear to be the right
> thing to do. We could hide the checkbox in releases (similar to the
> geolocation pref) and leave it off by default until the feature is
> considered mature enough for the general user.

Right. That's why I wrote: And possibly "browser.eme.ui.enabled" ....
This is a new groupbox added with the media.eme.enabled checkbox, retaining the current layout of the Scripts & Plugins prefpane. I've added it between the JavaScript and the Plugin options, given that it's like a plugin but in essence still part of HTML5. The groupbox label is similar to the other ones, and the checkbox label refers to CDMs (which I think is what's actually downloaded from the vendor when needed) rather than EME as the mechanism. I'm also emphasizing "Third-party" in that label to convey unambiguously that this part is not a native function of SeaMonkey.

At least on Windows, there isn't sufficient vertical space to show all checkboxes of the JavaScript permissions window. On the other hand, some space is wasted in the JavaScript and especially in the Plugin groupboxes.
With a couple of minor modifications, all six JavaScript permission options can be made visible without the need for a scrollbar:

 - JavaScript: removed the separator and in turn indented label and listbox,
   which makes logically more sense given that the options depend on the global
   preference being enabled;

 - Plugins: moved the "Suite" and "Mail & Newsgroups" checkboxes into an hbox,
   while retaining the logic that disabling the Suite as such for plugins will
   also disable the Mail & Newsgroups option; I think that's still intuitive.
Attached patch Proposed patch (obsolete) — Splinter Review
This implements attachment 8561049 [details] with the new groupbox not included for release builds similar to bug 994093 attachment 8414447 [details] [diff] [review] until all dependencies to the EME tracking bug 1015800 are satisfied.

Since I don't have access to bug 1095734, there is no Help content yet. Given the complexity of the issue and user concerns, this might be better deferred to either the bug enabling the checkbox and/or flipping the default to enabling EME by default (i.e., either way, another bug following up on this one).
Attachment #8561058 - Flags: ui-review?(neil)
Attachment #8561058 - Flags: review?(iann_bugzilla)
Neil, Ian: Any opinion on this?
As hinted in today's status meeting, I'm separating the definition of the preference from the UI patch to get this reviewed and checked-in asap (this will also have to land on comm-aurora).
Attachment #8578688 - Flags: review?(iann_bugzilla)
This is otherwise identical with attachment 8561058 [details] [diff] [review].

Further bugs to watch for any notifications/indications, but those seem to be orthogonal to the preferences UI introduced here:

[Bug 1089871] [EME] Provide an indication that protected content is being played back
[Bug 1089873] [EME] Provide links from indicators to additional information about Digital Rights Management Systems and protected content
[Bug 1089882] [EME] Provide links from CDMs on Addon->Plugins page to additional information about Digital Rights Management Systems and protected content
[Bug 1089883] [EME] Indicate when playback is not possible due to a disabled CDM
[Bug 1089885] [EME] Indicate when media playback is not possible due to an unsupported CDM
[Bug 1089886] [EME] Provide an indication that protected content is being played back in the media element context menu
[Bug 1089888] [EME] Allow management of links to information about DRM and protected content

Some of those may be Toolkit and thus would be inherited by SeaMonkey automagically.
Attachment #8561058 - Attachment is obsolete: true
Attachment #8561058 - Flags: ui-review?(neil)
Attachment #8561058 - Flags: review?(iann_bugzilla)
Attachment #8578695 - Flags: ui-review?(neil)
Attachment #8578695 - Flags: review?(iann_bugzilla)
Comment on attachment 8578688 [details] [diff] [review]
Part I: Add the preference

This part now has to land on comm-beta and comm-aurora as well before the next merge. Given that the one issuing branch approvals is the same person the review was requested from, I'm flagging this for branch approval in parallel. 

[Approval Request Comment]
Regression caused by (bug #): Core bug 1089876
User impact if declined: undefined preference
Testing completed (on m-c, etc.): works on c-c prior to requesting review
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: none
Attachment #8578688 - Flags: approval-comm-beta?
Attachment #8578688 - Flags: approval-comm-aurora?
Attachment #8578688 - Flags: review?(iann_bugzilla)
Attachment #8578688 - Flags: review+
Attachment #8578688 - Flags: approval-comm-beta?
Attachment #8578688 - Flags: approval-comm-beta+
Attachment #8578688 - Flags: approval-comm-aurora?
Attachment #8578688 - Flags: approval-comm-aurora+
Comment on attachment 8578695 [details] [diff] [review]
Part II: Add the UI, disabled for releases

>+++ b/suite/common/pref/pref-scripts.xul
>+      <label control="AllowList" class="indent"
Nit: one attribute per line.
>+             value="&allowScripts.label;"
>+             accesskey="&allowScripts.accesskey;"/>
r=me with that addressed.
Attachment #8578695 - Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
Whiteboard: [c-n: push #8578688 for comm-central/aurora/beta]
Thanks, comment #10 addressed.
Attachment #8578695 - Attachment is obsolete: true
Attachment #8578695 - Flags: ui-review?(neil)
Attachment #8591400 - Flags: ui-review?(neil)
Attachment #8591400 - Flags: review+
Whiteboard: [c-n: push #8578688 for comm-central/aurora/beta] → [leave open][c-n: push #8578688 for comm-central/aurora/beta]
(In reply to Philip Chee from comment #12)
> Comment on attachment 8578688 [details] [diff] [review]
> Part I: Add the preference
> http://hg.mozilla.org/comm-central/rev/556b4ad19790
http://hg.mozilla.org/releases/comm-aurora/rev/976e3ac4f9d9
http://hg.mozilla.org/releases/comm-beta/rev/f13db5b30387
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [leave open][c-n: push #8578688 for comm-central/aurora/beta] → [leave open]
Target Milestone: --- → seamonkey2.37
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: seamonkey2.37 → ---
A little bummer - according to bug 1150520 we may need to hide the groupbox depending on the platform (currently not supported for Linux and Mac OSX, but also Windows XP). That will likely imply some JavaScript magic, where a pointer to it is provided in bug 1150520 comment #1:

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPUtils.jsm#48
Upon a closer look, Firefox hides the box based on their browser.eme.ui.enabled in the firefox.js default preferences. Thus, a simple #ifdef XP_WIN around the #ifndef RELEASE_BUILD would do the same here for the time being. It's a bit more tricky with the sub platform, an example of which was checked in with mozilla-central changeset 5c603490ffb9 (where SeaMonkey doesn't seem to have a corresponding doorhanger notifications yet).

On the other hand, if the box is strictly removed for platforms other than Windows, users who want to test the feature on Linux or Mac OSX to the extent it /may/ be introduced in the future would have to resort to about:config to do that. Thus, my suggestion would be to go with just the RELEASE_BUILD preprocessor switch for now and change that to the actual platforms supported in beta and release builds later once dependencies listed in comment #8 have been ported.
Status: REOPENED → NEW
Flags: needinfo?(iann_bugzilla)
Status: NEW → ASSIGNED
Per discussion in today's status meeting, IanN would prefer that Neil makes the call whether or not to expose the EME preference for testing in the nightly builds at this time, thus switching the request.
Flags: needinfo?(iann_bugzilla) → needinfo?(neil)
Blocks: 1185980
(In reply to rsx11m from comment #4)
>  - Plugins: moved the "Suite" and "Mail & Newsgroups" checkboxes into an hbox,
Um, why?
To make sure that it fits vertically - compare attachment 8561048 [details] vs. attachment 8561049 [details].
Comment on attachment 8591400 [details] [diff] [review]
Part II: Add the UI, disabled for releases (v2)

>     <groupbox id="pluginPreferences">
>       <caption label="&enablePlugins.label;"/>
>+      <hbox id="enablePluginsRow">
Oh, in that case just use <groupbox orient="horizontal">.
Attachment #8591400 - Flags: ui-review?(neil) → ui-review+
(In reply to rsx11m from comment #16)
> Per discussion in today's status meeting, IanN would prefer that Neil makes
> the call whether or not to expose the EME preference for testing in the
> nightly builds at this time, thus switching the request.

(Huh, I don't see any mention in the meeting notes...) How do I test it?
Good question - quite likely it would require downloading some CDM like the Adobe one for handling the "protected" contents. That should only happen once the pref is enabled, but doesn't do anything for me so far. Apparently something is missing? Maybe there is a test page somewhere, but I can't find any.
(In reply to neil@parkwaycc.co.uk from comment #20)
> (Huh, I don't see any mention in the meeting notes...)

http://logs.glob.uno/?c=mozilla%23seamonkey&s=12+May+2015&e=12+May+2015#c641122
(In reply to rsx11m from comment #21)
> Good question - quite likely it would require downloading some CDM like the
> Adobe one for handling the "protected" contents. That should only happen
> once the pref is enabled, but doesn't do anything for me so far.

Well, my Windows 64-bit Firefox self-build has already downloaded an Adobe CDM, which surprised me. In the mean time I guess there's no point enabling the UI until we know it might actually do something.
(In reply to neil@parkwaycc.co.uk from comment #19)
> Comment on attachment 8591400 [details] [diff] [review]
> Part II: Add the UI, disabled for releases (v2)
> 
> >     <groupbox id="pluginPreferences">
> >       <caption label="&enablePlugins.label;"/>
> >+      <hbox id="enablePluginsRow">
> Oh, in that case just use <groupbox orient="horizontal">.

That simplifies things, especially as I only have to modify it in the overlay to get the desired alignment (thus replacing the indent with the groupbox orient) and can let the main element unchanged.

(In reply to neil@parkwaycc.co.uk from comment #23)
> Well, my Windows 64-bit Firefox self-build has already downloaded an Adobe
> CDM, which surprised me. In the mean time I guess there's no point enabling
> the UI until we know it might actually do something.

Firefox, yes - but SeaMonkey too? (I was testing on Linux, thus nothing may happen there yet.)
Well, here is the updated patch, let me know whether this can be checked in or not at this time.
Since it's for testing purposes only, it won't be visible in release builds anyway = no harm done.
Attachment #8591400 - Attachment is obsolete: true
Attachment #8639339 - Flags: ui-review+
Attachment #8639339 - Flags: review+
There is media.gmp-manager.url;https://aus4.mozilla.org/update/3/GMP/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml which would imply that the download information is coming from an AUS server, right? So, if it's not configured for SeaMonkey, nothing is downloaded. This would make sense.

http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/init/all.js#4906

Maybe an "I'm Firefox" URL can be faked to force download on SeaMonkey for testing?
(In reply to rsx11m from comment #26)
> media.gmp-manager.url;https://aus4.mozilla.org/update/3/GMP/%VERSION%/
> %BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/
> %DISTRIBUTION_VERSION%/update.xml which would imply that the download
> information is coming from an AUS server, right?

I opened the decoded URL in Firefox (just to be sure) but it only seems to be for the H264 video codec needed for webrtc; setting the pref in SeaMonkey only seemed to trigger some additional prefs related to that codec.
I see media.gmp-eme-adobe.* (in parallel to media.gmp-gmpopenh264.*) in Firefox 39.0 on Windows, which is interestingly defined in mozilla/browser/app/profile/firefox.js#1886. If AUS expects a matching Build ID in media.gmp-manager.buildID, it would be different in SeaMonkey vs. Firefox (but then you shouldn't see the H264 plugin either, unless it's less restrictive than the Adobe plugin).
Bug 1134919 - add Adobe CDM version 1 metadata to AUS - would suggest that a link to Adobe's CDM download site is provided by the production AUS4 update server.
Neil, I plan to flag attachment 8639339 [details] [diff] [review] for checkin after the merge next week. I think that the situation here is similar to bug 994093 for the geo.enabled pref - the pref is there, not fully functional yet, we want a UI to toggle it once it is; thus, visible in pre-release versions but not in releases. No point in letting the UI patch bitrot.
Flags: needinfo?(neil)
Keywords: checkin-needed
Whiteboard: [leave open] → [c-n: #8639339 for comm-central]
(In reply to rsx11m from comment #30)
> I think that the situation here is similar to bug 994093

Fair enough.
Comment on attachment 8639339 [details] [diff] [review]
Part II: Add the UI, disabled for releases (v3)

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/ca0555116908
Missed the merge date! Sorry!
Comment on attachment 8639339 [details] [diff] [review]
Part II: Add the UI, disabled for releases (v3)

(In reply to Philip Chee from comment #32)
> Missed the merge date! Sorry!

[Approval Request Comment]
Regression caused by (bug #): Core bug 1089876
User impact if declined: undefined preference
Testing completed (on m-c, etc.): works on c-c prior to requesting review
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: two strings to pref-scripts.dtd
Attachment #8639339 - Flags: approval-comm-aurora?
Keywords: checkin-needed
Whiteboard: [c-n: #8639339 for comm-central]
Target Milestone: --- → seamonkey2.40
(In reply to Philip Chee from comment #33)
> Comment on attachment 8639339 [details] [diff] [review]
> Part II: Add the UI, disabled for releases (v3)
> 
> (In reply to Philip Chee from comment #32)
> > Missed the merge date! Sorry!

No problem, thanks!

> [Approval Request Comment]
> Regression caused by (bug #): Core bug 1089876
> User impact if declined: undefined preference

Actually, the pref was already checked in on all branches with attachment 8578688 [details] [diff] [review], this patch only adds the UI. Given that it's uncertain to which extent EME/DRM/CDM-download is working already, that may not be urgent enough to warrant a late-l10n situation for the localizers.
Comment on attachment 8639339 [details] [diff] [review]
Part II: Add the UI, disabled for releases (v3)

> Actually, the pref was already checked in on all branches with attachment
> 8578688 [details] [diff] [review], this patch only adds the UI. Given that
> it's uncertain to which extent EME/DRM/CDM-download is working already, that
> may not be urgent enough to warrant a late-l10n situation for the localizers.
Great! Thanks!

> +#ifndef RELEASE_BUILD
There is resource://gre/modules/AppConstants.jsm
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/modules/AppConstants.jsm

So in JS you could do:

document.getElementById("drmPreferences").hidden = AppConstants.RELEASE_BUILD
Attachment #8639339 - Flags: approval-comm-aurora?
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to Philip Chee from comment #35)
> document.getElementById("drmPreferences").hidden = AppConstants.RELEASE_BUILD

Yeah, the preprocessor variable conveys the temporary/work-in-progress status of the feature better IMO (people first look into the XUL code to find the checkbox, then would look up JS restrictions).
While the preference is there in 2.40, it seems to have no effect on whatsoever (tested on Windows with a Win32 build, as this seems to be the only platform that Fx supports right now).

So it seems as comment #34 was right: the EME/DRM/CDM-download does not happen - do we have a bug for that?
Tested on Windows 7 and 8.1, as XP is not supported.
(In reply to Adrian Kalla [:adriank] from comment #37)
> So it seems as comment #34 was right: the EME/DRM/CDM-download does not
> happen - do we have a bug for that?

I don't think so - can you file it for SeaMonkey if you can't find anything already pending?
Blocks: 1446199
You need to log in before you can comment on or make changes to this bug.