Closed Bug 1294649 Opened 4 years ago Closed 4 years ago

DRM UI is confusing on Linux

Categories

(Core :: Audio/Video: Playback, defect)

49 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 + fixed
firefox50 + fixed
firefox51 --- fixed

People

(Reporter: glandium, Assigned: cpearce)

References

Details

(Keywords: regression)

Attachments

(2 files)

After bug 1289634, now on beta and aurora, this is what is visible in the UI:

- In about:addons > Plugins, Widevine Content Decryption Module is "disabled" and "Never Activate"
- In about:preferences > Content, "Play DRM content" is checked.

This is confusing, because DRM, while activated, very much depends on the status of plugins, which is not made clear. And that widevine is "Never Activate" doesn't reflect the fact that there's supposed to be a prompt.

Add to that that the "Learn more" page is written for Windows/OSX users for whom DRM *is* enabled by default, and you've scared away Linux users (without mentioning the critical bugs I'll be receiving in Debian for "WTF Debian, remove that non-free DRM crap")
I agree. I was halfway through writing a patch for this, then got distracted.

We should really have the "Play DRM Content" unchecked by default on Linux.
Assignee: nobody → cpearce
Comment on attachment 8781359 [details]
Bug 1294649 - Ensure 'Play DRM Content' is unchecked by default on Linux.

https://reviewboard.mozilla.org/r/71798/#review69326
Attachment #8781359 - Flags: review?(gsquelart) → review+
Comment on attachment 8781360 [details]
Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.

https://reviewboard.mozilla.org/r/71800/#review69408

::: toolkit/mozapps/extensions/internal/GMPProvider.jsm:369
(Diff revision 1)
> -    if (this.appDisabled) {
> +    if (!this.isActive) {
>        this.uninstallPlugin();
>      } else {
>        AddonManagerPrivate.callInstallListeners("onExternalInstall", null, this,
>                                                 null, false);
>        AddonManagerPrivate.callAddonListeners("onInstalling", this, false);
>        AddonManagerPrivate.callAddonListeners("onInstalled", this);
>        this.checkForUpdates(GMP_CHECK_DELAY);
>      }
>      if (!this.userDisabled) {
>        this._handleEnabledChanged();
>      }

This feels odd. Why is this change necessary? AFAICT this basically means that if you enable EME globally but one of the plugins is user-disabled, it will now be uninstalled. While your explanation for the Linux case where we don't actually have it as part of the install package (in which case, why is it in the list at all?) make some sense, on other OSes where we do have the Adobe CDM that doesn't sound like the right thing? Fundamentally 'uninstall' and 'disable' are / should be different things. What am I missing?

On top of that, this initial if/else block now no longer 'matches' the `!this.userDisabled` check below. Should that also be updated?

Some comments here would probably help me grok why we need to do this in addition to the pref flips that already got r+'d...
Attachment #8781360 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch (PTO recovery mode) from comment #5)
> Comment on attachment 8781360 [details]
> Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.
> 
> https://reviewboard.mozilla.org/r/71800/#review69408
> 
> ::: toolkit/mozapps/extensions/internal/GMPProvider.jsm:369
> (Diff revision 1)
> > -    if (this.appDisabled) {
> > +    if (!this.isActive) {
> >        this.uninstallPlugin();
> >      } else {
> >        AddonManagerPrivate.callInstallListeners("onExternalInstall", null, this,
> >                                                 null, false);
> >        AddonManagerPrivate.callAddonListeners("onInstalling", this, false);
> >        AddonManagerPrivate.callAddonListeners("onInstalled", this);
> >        this.checkForUpdates(GMP_CHECK_DELAY);
> >      }
> >      if (!this.userDisabled) {
> >        this._handleEnabledChanged();
> >      }
> 
> This feels odd. Why is this change necessary? AFAICT this basically means
> that if you enable EME globally but one of the plugins is user-disabled, it
> will now be uninstalled.

That's the existing behaviour.

> While your explanation for the Linux case where we
> don't actually have it as part of the install package (in which case, why is
> it in the list at all?) make some sense, on other OSes where we do have the
> Adobe CDM that doesn't sound like the right thing? Fundamentally 'uninstall'
> and 'disable' are / should be different things. What am I missing?

Currently, all GMPs (except gmp-clearkey) are downloaded, and aren't part of the install package on all platforms.

 
> On top of that, this initial if/else block now no longer 'matches' the
> `!this.userDisabled` check below. Should that also be updated?

I will see if I can clear that up.
For the record, I asked Gijs out-of-band whether he was ok for me to switch reviewers to spohl here,  and he was ok with that.
Comment on attachment 8781360 [details]
Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.

https://reviewboard.mozilla.org/r/71800/#review69652

Looks good to me, thanks!
Attachment #8781360 - Flags: review?(spohl.mozilla.bugs) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bca6f19b9320
Ensure 'Play DRM Content' is unchecked by default on Linux. r=gerald
https://hg.mozilla.org/integration/autoland/rev/212d06d11b24
Don't show EME plugins in the plugin list that are disabled. r=spohl
I must remember to uplift.
Flags: needinfo?(cpearce)
Comment on attachment 8781359 [details]
Bug 1294649 - Ensure 'Play DRM Content' is unchecked by default on Linux.

Approval Request Comment
[Feature/regressing bug #]: Widevine Linux EME uplift
[User impact if declined]: On Linux, the DRM/EME UI will be confusing; we'll have the "Play DRM Checkbox" on the preferences pane checked and the Widevine CDM will be listed as "never activate" in the add-ons > plugins pane. This is confusing as we can't actually play DRM content without the CDM enabled. With this patch, we'll not have the "Play DRM Checkbox" checked, so it's much clearer what's going on.
[Describe test coverage new/current, TreeHerder]: We have browser chrome tests covering this I believe.
[Risks and why]: Fairly low; we're basically changing a pref and the machinery around that.
[String/UUID change made/needed]: None
Flags: needinfo?(cpearce)
Attachment #8781359 - Flags: approval-mozilla-beta?
Attachment #8781359 - Flags: approval-mozilla-aurora?
Comment on attachment 8781360 [details]
Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.

Approval Request Comment
[Feature/regressing bug #]: Widevine Linux EME uplift
[User impact if declined]: On Linux, the DRM/EME UI will be confusing; we'll have the "Play DRM Checkbox" on the preferences pane checked and the Widevine CDM will be listed as "never activate" in the add-ons > plugins pane. This is confusing as we can't actually play DRM content without the CDM enabled. With this patch, we'll not have the "Play DRM Checkbox" checked, so it's much clearer what's going on.
[Describe test coverage new/current, TreeHerder]: We have browser chrome tests covering this I believe.
[Risks and why]: Fairly low; we're basically changing a pref and the machinery around that.
[String/UUID change made/needed]: None
Attachment #8781360 - Flags: approval-mozilla-beta?
Attachment #8781360 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/bca6f19b9320
https://hg.mozilla.org/mozilla-central/rev/212d06d11b24
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8781360 [details]
Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.

Recent regression, let's uplift to Aurora50.
Attachment #8781360 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8781359 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8781360 [details]
Bug 1294649 - Don't show EME plugins in the plugin list that are disabled.

Nice UI improvement for video playback, let's take this for the beta 6 build.
Attachment #8781360 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8781359 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Track 49+ as this is a regression.
Version: unspecified → 49 Branch
See Also: → 1404048
Firefox 57.0b4 and later is EME-free so no separate "linux-x86_64-EME-free" are distributed - https://bugzilla.mozilla.org/show_bug.cgi?id=1404048#c3

However, I just tested Firefox 59.0.2 64-bit for GNU/Linux:

- The Widevine Content Decryption Module plugin has been removed from about:addons > Plugins. -- This is good
- In about:preferences > Content, "Play DRM content" is unchecked. -- it can still be enabled so how does it make Firefox "EME-free"?
(In reply to public from comment #22)
> Firefox 57.0b4 and later is EME-free so no separate "linux-x86_64-EME-free"
> are distributed - https://bugzilla.mozilla.org/show_bug.cgi?id=1404048#c3
> 
> However, I just tested Firefox 59.0.2 64-bit for GNU/Linux:
> 
> - The Widevine Content Decryption Module plugin has been removed from
> about:addons > Plugins. -- This is good
> - In about:preferences > Content, "Play DRM content" is unchecked. -- it can
> still be enabled so how does it make Firefox "EME-free"?

It's EME-free in the same way that it's Flash-free, in that the browser doesn't come with it (ie the download does not contain any proprietary EME/DRM/Flash code), but if the user explicitly chooses to have it, the browser can use it (it'll download the CDMs at some point *if* the user chooses to flip the pref). Exactly the same thing happened with the separate EME-free builds that came before it - as https://bugzilla.mozilla.org/show_bug.cgi?id=1404048#c1 notes, all it did was default a pref to false. So nothing has really changed.
(In reply to :Gijs (he/him) from comment #23)
> (In reply to public from comment #22)
> > Firefox 57.0b4 and later is EME-free so no separate "linux-x86_64-EME-free"
> > are distributed - https://bugzilla.mozilla.org/show_bug.cgi?id=1404048#c3
> > 
> > However, I just tested Firefox 59.0.2 64-bit for GNU/Linux:
> > 
> > - The Widevine Content Decryption Module plugin has been removed from
> > about:addons > Plugins. -- This is good
> > - In about:preferences > Content, "Play DRM content" is unchecked. -- it can
> > still be enabled so how does it make Firefox "EME-free"?
> 
> It's EME-free in the same way that it's Flash-free, in that the browser
> doesn't come with it (ie the download does not contain any proprietary
> EME/DRM/Flash code), but if the user explicitly chooses to have it, the
> browser can use it (it'll download the CDMs at some point *if* the user
> chooses to flip the pref). Exactly the same thing happened with the separate
> EME-free builds that came before it - as
> https://bugzilla.mozilla.org/show_bug.cgi?id=1404048#c1 notes, all it did
> was default a pref to false. So nothing has really changed.

Thank you for that precise explanation.
You need to log in before you can comment on or make changes to this bug.