1331484 - Remove traces of Adobe from DecoderDoctor

Status: RESOLVED FIXED

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

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Spawned from bug 1329543 comment 40.
There are still mentions of the Adobe CDM in Decoder Doctor and related browser-media.js, they should be exterminated.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1331484 - Removed mentions of Adobe from browser-media.js -

We do not support Adobe Primetime anymore, so there is no need to handle
related issues.

Review commit: https://reviewboard.mozilla.org/r/109016/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109016/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

There was actually nothing related to Adobe in DecoderDoctorDiagnostics.
Handled what was left in browser-media.js and its tests.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8832771 [details]
Bug 1331484 - Removed mentions of Adobe from browser-media.js -

https://reviewboard.mozilla.org/r/109016/#review110262

We need to make a decision about the string here. Holding off on r+ for now because of that. :-)

::: browser/base/content/browser-media.js
(Diff revision 1)
>  
>  let gDecoderDoctorHandler = {
>    getLabelForNotificationBox(type) {
> -    if (type == "adobe-cdm-not-found" &&
> -        AppConstants.platform == "win") {
> -      return gNavigatorBundle.getString("decoder.noCodecs.message");

It seems this message reads:

"To play video, you may need to install Microsoft’s Media Feature Pack."

And has no other consumers besides the two you're removing here - though the decoder.noHWAcceleration.message string looks eerily similar.

Do we need to either add another case here so we still display this message to users who are on e.g. Win10 KN or N? Or does the other string take care of that already, in which case, should we remove https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/browser/locales/en-US/chrome/browser/browser.properties#842 ?

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8832771 [details]
Bug 1331484 - Removed mentions of Adobe from browser-media.js -

https://reviewboard.mozilla.org/r/109016/#review110262

> It seems this message reads:
> 
> "To play video, you may need to install Microsoft’s Media Feature Pack."
> 
> And has no other consumers besides the two you're removing here - though the decoder.noHWAcceleration.message string looks eerily similar.
> 
> Do we need to either add another case here so we still display this message to users who are on e.g. Win10 KN or N? Or does the other string take care of that already, in which case, should we remove https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/browser/locales/en-US/chrome/browser/browser.properties#842 ?

I intentionally left it there, because it seems like it could be a useful message, even if not used today. It's already been translated, so I didn't want to waste that effort in case we need it again one day.

Now, looking at the other message (starting with "to improve video quality"), it seems to be used in cases where we actually cannot play the requested media, so in fact we should probably be displaying the noCodecs "To play video..." messages!

So thank you for making me look, it seems I need to put more thought into this! However this is becoming off-topic with the current bug, which was just about removing Adobe-handling code.

How about:
- You r+ this, as (I think) it does the job of removing these Adobe bits, as per the bug description.
- I open a separate bug to review the whole Decoder Doctor decision tree, in which useless messages can be pruned.

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8832771 [details]
Bug 1331484 - Removed mentions of Adobe from browser-media.js -

(Please see comment 4)

Comment 6

[:Gijs (he/him)]

Comment on attachment 8832771 [details]
Bug 1331484 - Removed mentions of Adobe from browser-media.js -

https://reviewboard.mozilla.org/r/109016/#review110432

r=me given a followup bug to re-evaluate the strings and/or decision tree.

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you Gijs, I've opened bug 1336256 to follow up on the issue vs user report mismatch.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4290c796975c
Removed mentions of Adobe from browser-media.js - r=Gijs

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4290c796975c