1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable

Status: RESOLVED FIXED

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

Description

[Matt Woodrow (:mattwoodrow)]

Windows Vista and Windows 7/8 N don't ship with h264 codecs, but these can be downloaded from microsoft if the user wishes.

When a website tests for h264 availability using canPlayType() and the codec isn't present (but could be downloaded) we should show some sort of UI to the user telling them where to get it from.

Comment 1

[Chris Peterson [:cpeterson]]

Bug 1180108 is the meta bug for the UI and SUMO pages that will explain how to install the codecs. Let's use bug 848994 to track the platform work and use this bug to track the front-end UI work to notify the user.

Comment 2

[Chris Peterson [:cpeterson]]

Jared will be working on the front-end UI for the Decoder Doctor.

Comment 3

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

Jared, I hope you are not waiting for bug 848994 to land, as it may take yet a few more days to get it in tip-top shape!
If you are available to work on this soon, you should be able to start coding something based on my WIP: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3742f041fe653a4a19fd6149f53a7b6cd0edaedf
Please contact me if you need more information. (Here, email, IRC #media or #developers)

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: MozReview Request: Bug 1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable. r?gijs

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

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ba0d314472f

Comment 6

[:Gijs (he/him)]

Comment on attachment 8740186 [details]
MozReview Request: Bug 1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable. r?gijs

https://reviewboard.mozilla.org/r/45617/#review42241

::: browser/base/content/browser.js:2517
(Diff revision 1)
>      this.fxaBadge = null;
>      this._showBadge();
>    }
>  };
>  
> +var gDecoderDoctorNotification = {

Can you rename browser-eme.js to browser-media.js or browser-media-handling.js or something, and stick this in there, instead of further increasing the bulk of browser.js ? :-)

::: browser/base/content/browser.js:2527
(Diff revision 1)
> +    }
> +
> +    Services.obs.addObserver(this, "decoder-doctor-notification", false);
> +  },
> +
> +  get platform_cannot_play_message() {

Nit: please use camelCase like for the object on which this lives.

::: browser/base/content/browser.js:2529
(Diff revision 1)
> +    Services.obs.addObserver(this, "decoder-doctor-notification", false);
> +  },
> +
> +  get platform_cannot_play_message() {
> +    if (AppConstants.isPlatformAndVersionAtMost("win", "5.9")) {
> +      return "decoder.noCodecsXP.message";

I don't see strings being added in this patch?

::: browser/base/content/browser.js:2537
(Diff revision 1)
> +      return "decoder.noCodecsLinux.message";
> +    }
> +    return "decoder.noCodecs.message";
> +  },
> +
> +  observe(subject, topic, data) {

So... as best I can tell the observer notification in bug 1248507 is being fired from dom/media/ code and passes the inner window as a subject. This means that in e10s land, it's fired in the content process, no? This code is running in the parent, so I don't understand how it can work in e10s.

::: browser/base/content/browser.js:2542
(Diff revision 1)
> +        let url =
> +          Services.urlFormatter.formatURLPref("app.support.baseURL") +
> +          "fix-video-audio-problems-firefox-windows";
> +        openUILinkIn(url, "tab");

Nit:

let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
openUILinkIn(baseURL + "fix-video-audio-problems-firefox-windows", "tab");

::: browser/base/content/browser.js:2544
(Diff revision 1)
> +      label: gNavigatorBundle.getString("decoder.noCodecs.button"),
> +      accessKey: gNavigatorBundle.getString("decoder.noCodecs.accesskey"),
> +      callback() {
> +        let url =
> +          Services.urlFormatter.formatURLPref("app.support.baseURL") +
> +          "fix-video-audio-problems-firefox-windows";

Did this URL get coordinated with SUMO folks? It looks like the patch in bug 1248507 initially implements Linux support, so the URL seems wrong...

::: browser/base/content/browser.js:2556
(Diff revision 1)
> +    if (type == "cannot-play") {
> +      title = gNavigatorBundle.getString(gDecoderDoctorNotification.platform_cannot_play_message);
> +    } else {
> +      title = gNavigatorBundle.getString("decoder.noHWAcceleration.message");
> +    }
> +    let notificationBox = gBrowser.getNotificationBox();

This should use the notification box matching the (top of the, in the case of frames...) inner window on which the notification is fired, not just the current tab.

::: browser/base/content/browser.js:2559
(Diff revision 1)
> +    let previousNotification = notificationBox.getNotificationWithValue(value);
> +    if (previousNotification) {
> +      notificationBox.removeNotification(previousNotification);
> +    }

Does the notificationbox API not do this already? In any case, I believe that removeNotification is async if you don't pass `true` as the second parameter (aSkipAnimation). I don't think we want that in this case?

We could do with a test for this case, too.

::: browser/base/content/browser.js:2567
(Diff revision 1)
> +    }
> +
> +    notificationBox.appendNotification(
> +      title,
> +      value,
> +      "",

I'm assuming this shows the default image?

::: browser/base/content/test/general/browser_decoderDoctor.js:16
(Diff revision 1)
> +
> +    let notification;
> +    try {
> +      notification = yield awaitNotificationBar;
> +    } catch (ex) {
> +      ok(AppConstants.platform == "macosx", "Decoder doctor notifications aren't implemented on OSX");

Seems like we should skip-if the test on macosx, and bail out earlier instead of waiting for the notification bar...

::: browser/base/content/test/general/browser_decoderDoctor.js:19
(Diff revision 1)
> +    todo(AppConstants.platform == "macosx",
> +         "Decoder doctor notifications aren't implemented on OSX");

I'm not sure this is useful...

::: browser/base/content/test/general/browser_decoderDoctor.js:37
(Diff revision 1)
> +      Services.urlFormatter.formatURLPref("app.support.baseURL") +
> +      "fix-video-audio-problems-firefox-windows";
> +    let awaitNewTab = BrowserTestUtils.waitForNewTab(gBrowser, url);
> +    button.click();
> +    let sumoTab = yield awaitNewTab;
> +    BrowserTestUtils.removeTab(sumoTab);

You should yield for this.

::: browser/base/content/test/general/browser_decoderDoctor.js:42
(Diff revision 1)
> +    BrowserTestUtils.removeTab(sumoTab);
> +  });
> +}
> +
> +add_task(function* test_decoder_doctor_notification_cannot_play() {
> +  yield test_decoder_doctor_notification("cannot-play", gDecoderDoctorNotification.platform_cannot_play_message);

Relying on the implementation for its own test seems a little dodgy... can we just have the same logic in the test?

Comment 7

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

Comment on attachment 8740186 [details]
MozReview Request: Bug 1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable. r?gijs

https://reviewboard.mozilla.org/r/45617/#review42517

I am not qualified to review all of this code, but :gijs is doing just that (thank you).
I'm giving this a tentative "ship-it", so I'm not blocking shipping, and subject to your handling of at least the WMF-not-found notification (details below).

::: browser/base/content/test/general/browser_decoderDoctor.js:41
(Diff revision 1)
> +add_task(function* test_decoder_doctor_notification_cannot_play() {
> +  yield test_decoder_doctor_notification("cannot-play", gDecoderDoctorNotification.platform_cannot_play_message);
> +});

Just so you know, the most important notification to watch for is "WMF-not-found", see bug 848994 for progress on the back-end side, and [1] for the latest list of notifications. In summary:
- "WMF-not-found" is sent on Windows when none of the requested formats can play, and we cannot load the WMF libs.
- "platform-decoder-not-found" is sent on Linux when we cannot load the FFMpeg lib. Remember we cannot explicitly reference "FFMpeg" in the user notification.
- "cannot-play" is sent when no decoder can play the requested format(s). It's a more generic notification than the above one, and I'm not sure you should actually display anything in that case, and/or what to display, as it's not explicitly covered in the DecDoc UX document.
- "can-play-but-some-missing-decoders" is sent when pref "media.decoderdoctor.verbose"==true, and *some* of the requested formats cannot be played. This is more for debugging (or possibly website designers), and so I don't think you need to do anything with it, except for your own debugging if you like.

You should only receive one of the above notifications at once. However it is possible to receive more than one over the lifetime of a webpage, if it makes requests separated by more than ~2 seconds. So please make sure you handle this case and don't display more than one drop-down bar at a time!

[1] https://bugzilla.mozilla.org/attachment.cgi?id=8740761&action=diff#a/dom/webidl/DecoderDoctorNotification.webidl_sec2

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

This is a bit confusing.

1) What message is going to trigger the HW accelerated error? It's not clear in your above response.
2) WMF-not-found is Windows-centric in the name. Whereas "platform-decoder-not-found" is not platform centric, but is only sent on Linux. Why not use the same topic for both of these and then let the frontend decide which string to use based on the platform?
3) What would happen if "WMF-not-found" was seen on non-Windows? What would happen if "platform-decoder-not-found" was seen on non-Linux?
4) If "can-play-but-some-missing-decoders" is only a debug message, why send it to the frontend? Should we always dump this to the console in the frontend? The backend can dump it to the console too without sending it to the frontend.
5) Can you explain what you mean by sending multiple requests but separated by ~2 seconds? Are you coalescing these events on the backend and flushing them at minimums of 2 seconds? Should we be expecting to swap notification bars if the type changes between events? What is the priority to overwrite existing decoder-doctor notifications? For example, I would expect a "WMF-not-found" message to have higher priority than something like "can-play-but-some-missing-decoders". Would that also mean that if we got a "WMF-not-found" message followed by something like a "can-play-but-some-missing-decoders" that we should keep the "WMF-not-found" notification bar?

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

https://reviewboard.mozilla.org/r/45617/#review42241

> I don't see strings being added in this patch?

Strings were added in bug 1262556.

> Did this URL get coordinated with SUMO folks? It looks like the patch in bug 1248507 initially implements Linux support, so the URL seems wrong...

We aren't showing a Learn More button for Linux.

> Does the notificationbox API not do this already? In any case, I believe that removeNotification is async if you don't pass `true` as the second parameter (aSkipAnimation). I don't think we want that in this case?
> 
> We could do with a test for this case, too.

I changed this code to follow the style from gEMEHandler.showNotificationBar and just return early if the notification box already exists.

> I'm assuming this shows the default image?

Yes, it shows the (i) info graphic. I added a comment.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=78e0c9a6ae83

Comment 11

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

Chris, please comment if you have anything to suggest, especially about the "5)" part. Should you/we describe these special situations in your UX doc?


(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> This is a bit confusing.

Probably my fault!
I've re-read the public "Decoder Doctor UX" document, and I see that I deviated from it, sorry about that.
https://docs.google.com/document/d/1BUHV-kQ3Kf9fC614CTjPxz_PaNerlNJerMQ8-t3e3lM

> 1) What message is going to trigger the HW accelerated error? It's not clear
> in your above response.

"platform-decoder-not-found" on Vista+. Note: Not yet implemented as it should be!

> 2) WMF-not-found is Windows-centric in the name. Whereas
> "platform-decoder-not-found" is not platform centric, but is only sent on
> Linux. Why not use the same topic for both of these and then let the
> frontend decide which string to use based on the platform?
> 3) What would happen if "WMF-not-found" was seen on non-Windows? What would
> happen if "platform-decoder-not-found" was seen on non-Linux?

My mistake, I've now removed "WMF-not-found", and will instead send the other messages as per the UX doc. You'll have to decide what to display depending on the current OS.

> 4) If "can-play-but-some-missing-decoders" is only a debug message, why send
> it to the frontend? Should we always dump this to the console in the
> frontend? The backend can dump it to the console too without sending it to
> the frontend.

I was hoping this message (and "cannot-play") would help you debug your code, without having to resort to installing weird OS's.
But if you'd prefer me not to send these messages, I can easily remove them.
Note that I already take care of sending logs to the web console, so you shouldn't need to do that.

> 5) Can you explain what you mean by sending multiple requests but separated
> by ~2 seconds? Are you coalescing these events on the backend and flushing
> them at minimums of 2 seconds? Should we be expecting to swap notification
> bars if the type changes between events? What is the priority to overwrite
> existing decoder-doctor notifications? For example, I would expect a
> "WMF-not-found" message to have higher priority than something like
> "can-play-but-some-missing-decoders". Would that also mean that if we got a
> "WMF-not-found" message followed by something like a
> "can-play-but-some-missing-decoders" that we should keep the "WMF-not-found"
> notification bar?

This is an implementation detail to handle some websites that make many codec checks together!

Based on a strict interpretation of the UX doc, I should send you a message *every* time a website does a codec check (through video.canPlayType() or MediaSource.isTypeSupported()).

But some websites (e.g. YouTube) make about 20 codec checks altogether, to see what the browser is capable of, before actually using one particular codec for the actual playback. Because there is no easy way to know when the website has finished doing these checks, I start an (arbitrary) 1-second timer from the first check, then gather all the incoming checks together, and after that 1 second I decide whether the website as a whole can or cannot play any video (assuming it will use whatever "good" codec is available), and send the appropriate message to the front-end.

The risk is that another codec check will happen after that first 1-second period, in which case I might send another message based on further checks.
Now, a "reasonable" website should do the right thing, and so if all codecs failed (and I would have sent you a "platform-decoder-not-found" or similar) it won't try to play anything, and nothing more should happen; or if some codecs failed and others worked (and I would send you no messages), it won't try to play with one of the failing codecs.
In the end, I wouldn't expect multiple messages (or at least multiple *different* messages) to be sent.

However we don't live in a perfect world, so you should just be ready for a nonsensical succession of messages, and just do what you think is best, e.g. blindly display the latest one, or display the "worst" one seen so far.

Comment 12

[Chris Peterson [:cpeterson]]

> > 2) WMF-not-found is Windows-centric in the name. Whereas
> > "platform-decoder-not-found" is not platform centric, but is only sent on
> > Linux. Why not use the same topic for both of these and then let the
> > frontend decide which string to use based on the platform?
> > 3) What would happen if "WMF-not-found" was seen on non-Windows? What would
> > happen if "platform-decoder-not-found" was seen on non-Linux?
> 
> My mistake, I've now removed "WMF-not-found", and will instead send the
> other messages as per the UX doc. You'll have to decide what to display
> depending on the current OS.

Yes. For example, "platform-decoder-not-found" would generate noHWAcceleration on Vista+ and noCodecsLinux on Linux.

The "Show Notification Bar" section of the UX doc describes how to map the events to UI strings for each platform. I just added a table to the doc that summarizes the message mappings for each OS. Hopefully that will make things clearer.

https://docs.google.com/document/d/1BUHV-kQ3Kf9fC614CTjPxz_PaNerlNJerMQ8-t3e3lM/


> Based on a strict interpretation of the UX doc, I should send you a message
> *every* time a website does a codec check (through video.canPlayType() or
> MediaSource.isTypeSupported()).
> 
> But some websites (e.g. YouTube) make about 20 codec checks altogether, to
> see what the browser is capable of, before actually using one particular
> codec for the actual playback. Because there is no easy way to know when the
> website has finished doing these checks, I start an (arbitrary) 1-second
> timer from the first check, then gather all the incoming checks together,
> and after that 1 second I decide whether the website as a whole can or
> cannot play any video (assuming it will use whatever "good" codec is
> available), and send the appropriate message to the front-end.

When I wrote the UX doc, we knew that websites like YouTube make multiple codec checks. We obviously don't want to show 20 info bars, but I don't know whether it would be better to coalesce the info bar events in the C++ code or the front-end JS. It sounds like Gerald implemented this with a timer in C++. This sounds good to me because it would reduce the event traffic and the event-firing logic is mostly contained in C++.


> The risk is that another codec check will happen after that first 1-second
> period, in which case I might send another message based on further checks.
> Now, a "reasonable" website should do the right thing, and so if all codecs
> failed (and I would have sent you a "platform-decoder-not-found" or similar)
> it won't try to play anything, and nothing more should happen; or if some
> codecs failed and others worked (and I would send you no messages), it won't
> try to play with one of the failing codecs.

We should expect some websites to fall back to Flash video. Vimeo does this. But we still want to show the info bar because we would prefer to play HTML5 video instead of Flash.

However, XP users would probably get a better H.264 video performance with Flash than Adobe's (software-only) CDM. But figuring out whether the page fell back to Flash video gets complicated, so we don't try.


> However we don't live in a perfect world, so you should just be ready for a
> nonsensical succession of messages, and just do what you think is best, e.g.
> blindly display the latest one, or display the "worst" one seen so far.

Jared, you can implement whatever is easiest for you. First message wins is probably simplest.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8740186 [details]
MozReview Request: Bug 1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable. r?gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45617/diff/1-2/

Comment 14

[:Gijs (he/him)]

Comment on attachment 8740186 [details]
MozReview Request: Bug 1160424 - [Decoder Doctor] Show notification UI when a requested codec is missing but downloadable. r?gijs

https://reviewboard.mozilla.org/r/45617/#review44423

r=me with the below addressed. Thanks for the test, that looks to be helpful.

::: browser/base/content/browser-media.js:180
(Diff revision 2)
>  
>  XPCOMUtils.defineLazyGetter(gEMEHandler, "_brandShortName", function() {
>    return document.getElementById("bundle_brand").getString("brandShortName");
>  });
>  
> +var gDecoderDoctorHandler = {

I'm assuming nobody relies on being able to do window.gDecoderDoctorHandler, so should this be `let` ? I don't really mind either way, so not opening an issue for this, but I figured I'd flag it up.

::: browser/base/content/browser-media.js:182
(Diff revision 2)
> +    return type == "adobe-cdm-not-found" ||
> +           type == "adobe-cdm-not-activated" ||
> +           type == "platform-decoder-not-found";

Some of these only work on some platform / version combinations. So in principle this can now show notifications with no label, because we don't check for an empty `title` variable as returned from `getLabelForNotificationBox`.

It seems like the simplest solution here would be to omit this method, call `getLabelForNotificationBox` earlier, and early return if it returns the empty string. Is there a reason not to do that?

::: browser/base/content/browser-media.js:188
(Diff revision 2)
> +           type == "adobe-cdm-not-activated" ||
> +           type == "platform-decoder-not-found";
> +  },
> +
> +  shouldShowLearnMoreButton() {
> +    return AppConstants.platform != "linux";

Should this be `== "win"` ? Why (not) ?

(My thinking being that we don't currently use this on OSX, but if we did, it seems the current learn more link is hardcoded to a Windows-specific SUMO article...)

::: browser/base/content/browser-media.js:195
(Diff revision 2)
> +
> +  getLabelForNotificationBox(type) {
> +    if (type == "adobe-cdm-not-found" &&
> +        AppConstants.platform == "win") {
> +      if (AppConstants.isPlatformAndVersionAtMost("win", "5.9")) {
> +        // We supply our own Learn More link so we don't need to populate the message here.

nit: s/link/button/

(I was confused for a bit as to where this provisioning happened... :-) )

::: browser/base/content/browser-media.js:196
(Diff revision 2)
> +  getLabelForNotificationBox(type) {
> +    if (type == "adobe-cdm-not-found" &&
> +        AppConstants.platform == "win") {
> +      if (AppConstants.isPlatformAndVersionAtMost("win", "5.9")) {
> +        // We supply our own Learn More link so we don't need to populate the message here.
> +        return gNavigatorBundle.getFormattedString("emeNotifications.drmContentDisabled.message", [""]);

This seems a little unfortunate; are we sure that this string will work in other locales when passed an empty string for the link contents?

::: browser/base/content/browser-media.js:232
(Diff revision 2)
> +      parsedData = JSON.parse(data);
> +    } catch (ex) {
> +      Cu.reportError("Malformed Decoder Doctor message with data: " + data);
> +      return;
> +    }
> +    let {type: type} = parsedData;

Nit: I *think* you can just do `let {type} = parsedData`, no?

Comment 15

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

The latest piece of the Decoder Doctor back-end puzzle has just landed in inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e07d236d6566

Tries looked good so hopefully it will stick and should ride with 48 (with potential uplift to 47 later on, TBD). I hope you can join me there!

Of particular interest for you is that last patch, which talks about the pref used to filter notifications going to the front-end.
By default, it's restricted to one very specific case, so for your testing you may want to set "media.decoder-doctor.notifications-allowed" to "*" to get notifications in simpler tests. Feel free to contact me if you would like further help with that.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for check-in

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

https://reviewboard.mozilla.org/r/45617/#review44423

> I'm assuming nobody relies on being able to do window.gDecoderDoctorHandler, so should this be `let` ? I don't really mind either way, so not opening an issue for this, but I figured I'd flag it up.

Yeah, we can use let here, though it may confuse people in the future. I've made the switch.

> This seems a little unfortunate; are we sure that this string will work in other locales when passed an empty string for the link contents?

I checked l10n-central. This string has %S on the right in all locales except Marathi, a left-to-right language, where it is seen at the beginning of the string with no punctuation separating the Learn more link. I translated the Marathi string using Google Translate and it doesn't self-reference the Learn More link. Note too that the string is on the right for the RTL languages such as Hebrew and Arabic. Neither Hebrew nor Arabic self-reference the Learn More link.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Gerald and Chris, can one of you test the attached patch to see that the notification bars show up? I can't trigger them on my local Windows 10 machine with the patches from bug 848994. I have the following prefs set:
media.decoderdoctor.verbose=true
media.decoderdoctor.enable-notification-bar=true
media.decoder-doctor.notifications-allowed=*
media.wmf.enabled=false

I loaded http://pearce.org.nz/video/h264.html and the video plays. With MSVC2015 attached, I hit a breakpoint in DecoderDoctorDiagnostics::SynthesizeAnalysis but canPlay gets set to `true` and the "DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, decoders available for all requested formats" message is dumped through DD_DEBUG.

Comment 19

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #18)
> Gerald and Chris, can one of you test the attached patch to see that the
> notification bars show up? I can't trigger them on my local Windows 10
> machine with the patches from bug 848994. I have the following prefs set:
> media.decoderdoctor.verbose=true
Not really needed, but doesn't hurt.

> media.decoderdoctor.enable-notification-bar=true
Obsolete, it's so last Thursday!

> media.decoder-doctor.notifications-allowed=*
Yep.

> media.wmf.enabled=false
Unfortunately, that last one doesn't quite work as Chris thought it would.
It blocks use of WMF in some places. However DecoderDoctor detects when WMF *is* enabled but fails to load!

> I loaded http://pearce.org.nz/video/h264.html and the video plays. With
> MSVC2015 attached, I hit a breakpoint in
> DecoderDoctorDiagnostics::SynthesizeAnalysis but canPlay gets set to `true`
> and the "DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() -
> Can play media, decoders available for all requested formats" message is
> dumped through DD_DEBUG.
Also, you probably have the Adobe Primetime, which takes over when WMF is not available, that's why you see canPlay==true.

If you'd like to fudge the test, you can easily add a line of code to get what you want, in
dom/media/platform/PDMFactory.cpp:
> @@ -297,6 +297,8 @@ PDMFactory::CreatePDMs()
>    if (sWMFDecoderEnabled) {
>      m = new WMFDecoderModule();
>      if (!StartupPDM(m)) {
>        mWMFFailedToLoad = true;
>      }
> +  } else {
> +    mWMFFailedToLoad = true;
>    }

Also you can get more Decoder Doctor analysis details by running Firefox with:
> NSPR_LOG_MODULES=DecoderDoctor:5

Anyway, your patch works as expected (on both e10s and non-e10s), so it's good to go!

Comment 20

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

Well, almost perfect... Please add the missing '/kb' in the SUMO URL, and then it will be good to go.

Comment 21

[:Gijs (he/him)]

(In reply to Gerald Squelart [:gerald] from comment #20)
> Well, almost perfect... Please add the missing '/kb' in the SUMO URL, and
> then it will be good to go.

SUMO should have a redirect from the link that Firefox produces with the current code. See other code:

https://dxr.mozilla.org/mozilla-central/search?q=format+app.support.baseurl&redirect=false&case=false

We should not need to manually include "kb".

Comment 22

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

Great, let's get that code landed then!

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0debbed8046d

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0debbed8046d