MediaSession enabled all builds from FF82 #6845

hamishwillee · 2020-10-06T05:08:58Z

MediaSession (and friends) are enabled on all builds from FF82 - this adds the version. See bug Enable MediaSession API

The fix enables everything that is conditional on dom.media.mediasession.enabled so set covered is everything that was previously tagged with that preference.
I have not added this for FF Android, as this was feature not previously supported for FF Android even via preference (am querying to make sure this is correct).
The spec is still marked as draft so I did not change the experimental flag.

ddbeck · 2020-10-08T17:09:31Z

I have not added this for FF Android, as this was feature not previously supported for FF Android even via preference (am querying to make sure this is correct).

@hamishwillee I've marked this as info-needed and not-ready, pending an answer on this bit. Or we can move on to reviewing without it, if you want to open an issue to track that question separately. Thank you!

hamishwillee · 2020-10-08T22:25:37Z

@ddbeck On a closely related matter, would appreciate your advice. Navigator.mediaSession is how the spec says you get hold of a MediaSession. So to my mind there really is no way the compatibility for these two objects should not match. You wouldn't invent a non-spec way to access everything else 'according to spec'.

I think that probably Navigator.mediaSession just wasn't updated last time the mediaSession was.

So I propose to also make these match. Reasonable?

hamishwillee · 2020-10-08T22:55:17Z

@ddbeck I asked the engineer about this change on FF.

He says that a bunch of pages can use the media control API and it is up to the user agent (device) to select which one gets used in the lock screen (etc). However right now Fenix is still using their own android component and so none of the sessions will be selected.

Doesn't quite feel "right", but based on his advice I plan to add it as version-supported in FF82 on Android. I thought perhaps a note like below - reasonable?

"notes": "The particular Media Session used for Android media control depends on the user agent. The Android browser implementation does not allow any session to be selected as active".

chrisdavidmills · 2020-10-12T16:12:06Z

@hamishwillee this looks OK to me; I'd move forward with that change.

chrisdavidmills · 2020-10-12T16:18:46Z

@hamishwillee in the list of files updated, I don't see Navigator listed. Surely Navigator.mediaSession should also be updated to show support from Fx version 82?

chrisdavidmills · 2020-10-12T16:25:06Z

@hamishwillee I also noticed that this PR does not include data for the MediaPositionState and MediaSessionActionDetails — those pages don't seem to have any browser-compat-data at all. Can you add it as part of this PR?

hamishwillee · 2020-10-12T22:34:31Z

@hamishwillee in the list of files updated, I don't see Navigator listed. Surely Navigator.mediaSession should also be updated to show support from Fx version 82?

@chrisdavidmills That was my question in #6845 (comment) - I think it was missed last time MediaSession got updated. I've made MediaSession and Navigator.mediaSession match as much as sensibly possible.

hamishwillee · 2020-10-12T23:36:37Z

@hamishwillee I also noticed that this PR does not include data for the MediaPositionState and MediaSessionActionDetails — those pages don't seem to have any browser-compat-data at all. Can you add it as part of this PR?

@chrisdavidmills @ddbeck I added MediaPositionState by pulling in #6002 and updating with the new information + some minor fixes

MediaSessionActionDetails was already documented here and table is in the docs:

@ddbeck This should be ready to review

hamishwillee · 2020-10-26T05:52:19Z

@ddbeck This is just a reminder. I am not nagging. Honest :-)

ddbeck · 2020-10-26T14:27:15Z

@hamishwillee Thanks for the nudge. I was more or less out last week, so it might take me a little time to catch up on things. I started on reviewing this before, but the backstory (e.g., #6002) is so complicated and angsty that it's slowing going. Thanks for your patience!

hamishwillee · 2020-11-08T22:46:37Z

Still not nagging :-).

ddbeck

Thanks for your reminders, @hamishwillee. The actual version data looks unobjectionable. 👍 There are a couple of structural hurdles here, however (not your fault—just a complexity we have to face here). I've got a couple big line comments to get started on those. Thanks again!

ddbeck · 2020-11-11T16:20:00Z

api/MediaMetadata.json

          "firefox_android": {
-            "version_added": null
+            "version_added": "82",
+            "notes": "The particular Media Session used for Android media control depends on the user agent. The Android browser implementation does not allow any session to be selected as active."


I'm having a hard time understanding this note. I ended up with a bunch of questions.

What does "depends on the user agent" mean here? Isn't Firefox the user agent? What about "does not allow any session to be selected as active"? This feature is the MediaMetadata interface itself. How does one use the interface to "select" something?

I need some help here to figure out how to translate this into something that a web developer can act on. MediaMetadata apparently has some limitation on Android. As developer, what can I do with it? What can't I do with it? How is it like other implementations? How is it different?

I see now that this note appears in several places. When is it most important to developers? When are they most likely to notice the behavior on Android?

@ddbeck What this says is that the API has been implemented but really can't be used. However it code running in a page that uses this API will never know that. Explanation:

The theory is that pages use this API to be able to expose media data and control to a user agent. In theory a user should be able to access an interface to select from pages using the API (and other media sources) and set one as being used by standard mechanisms for media display/control on the device. In other words, if your device supports playing media from the lockscreen (and displaying the current song say) then you should be able to select one of the pages as the source for media.

But what has happened is that the API was implemented, but the components required to make that UI selection to assign a page to the lockscreen (say) do not work/exist. So the API "works" - you can write a page that uses it. But then you still can't select the page.

An subtle point here though is that many pages might use the API, and most of them will not be selected by the user agent. No page actually knows if it is being used. There is nothing a developer can do about this.

The text is what the developer thought made sense to cover this. It is strictly accurate. But perhaps we should be blunt and say - there is no point using this?

Maybe something more like (?):

At time of writing the Android browser implementation does not allow web pages to be set as a media source.

Ah, OK. Thank you, @hamishwillee! That's really helpful. I think this helps me understand what's going on here a bit better. I think we need to revise the note and tweak the data a bit.

I think your draft note is pretty good start. To help this note standalone better in multiple contexts (MDN's not the only BCD consumer), does something like this revision work?

Firefox does not implement a user interface for choosing a web page as a media source. This API is exposed, but has no corresponding user-facing behavior.

That seems like a big enough caveat that we'd also want to set "partial_implementation": true.

Firefox does not implement a user interface for choosing a web page as a media source. This API is exposed, but has no corresponding user-facing behavior.

I will go back and check with engineering because it may be that the missing part of the picture is outside of the browser/in the OS; in which case this feels wrong. That then would also make "partial_implementation": true feel wrong.

However FMI, if we did do this, would the note exist on every API? I. not clear to me whether the note would only perhaps go on the main Session and Action.

@ddbeck An excellent description of the integration added here. In summary, it is as we thought - this API is integrated into GeckoView, but anything that uses GeckoView must further integrate with the underlying platform via the embedding application - in this case Firefox Android.

I have therefore taken your advice and changed to partial implementation and added note as below in all locations.

Firefox does not yet integrate this API into Android's media control interfaces. Hence the API is exposed, but the corresponding user-facing behavior is not available.

Should be good to review again.

ddbeck · 2020-11-11T16:53:48Z

api/MediaPositionState.json

@@ -0,0 +1,248 @@
+{
+  "api": {
+    "MediaPositionState": {


OK, this steps into a complex question about whether and how to represent IDL dictionaries in BCD. In this case, since, as far as I can tell, MediaPositionState only matters for one method, we should move the subfeatures of MediaPositionState to api.MediaSession.setPositionState (e.g., described as "<code>state.duration</code> argument") and get rid of this file.

Now, why we should do this pretty complex. I'm sorry about this. If you're really curious, you can read #6002 and #6265, but it's a long story. Here's my highly-opinionated summary:

The Media Session spec names several abstract dictionaries as a convenience to spec authors and browser developers. However, these dictionary names don't have a concrete manifestation for web developers; they indirectly appear as patterns for concrete return values and method arguments. Consequently, it's an open question what it means to "support" an abstract dictionary.

That said, we don't have a good answer in BCD or on MDN for how to represent these things yet. There's way more discussion on this point in #7287. In the past (e.g., #6265), I've compromised when such dictionaries have touched many APIs, but I've resisted introducing them in cases like this one.

As far as I can tell, MediaPositionState represents one thing only: the structure of the object argument to MediaSession.setPositionState(). I don't know what it means to support MediaPositionState, but I do have a sense for what it means for setPositionState() to support state.duration. We can represent this information as subfeatures of the method and call it a day.

hamishwillee · 2020-11-13T01:11:34Z

api/MediaSession.json

@@ -270,7 +274,8 @@
              }
            ],
            "firefox_android": {
-              "version_added": false
+              "version_added": "82",


@ddbeck With respect to your comment on MediaPositionState here: #6845 (comment)

I have deleted the specific doc for the MediaPositionState. There is no code change here, so arguably we could just ignore adding anything for this - I added it because I though it was an omission. If we do need subfeature documentation, could you point me at a good example of how this would be done in BCD that I can copy.

A quick search turned up something like api.HTMLOrForeignElement.focus.preventScroll_option (MDN). The equivalent here would be something like api.MediaSession.setPositionState.{duration,playbackRate,position}, or, to preserve the top-level MediaPositionState, api.MediaSession.setPositionState.MediaPositionState.{duration,playbackRate,position}.

Also an unrelated heads up: you don't need to rebase PRs on BCD. We squash most merges anyway.

Thanks @ddbeck . I think I've done this bit "as expected" now. Note again that I've copied in the same note as we discussed above - and I'm not sure this is appropriate within this level of nesting.

ddbeck

OK, I think we're getting pretty close here. Some hopefully straight-forward changes to make to get this in. Thank you, @hamishwillee for helping to fix up this (sometimes agitating) set of features.

api/MediaMetadata.json

ddbeck · 2020-11-25T13:04:47Z

api/MediaSession.json

+        },
+        "MediaPositionState": {
+          "__compat": {
+            "description": "Dictionary passed to <code>setPositionState</code> to specify play position within a track (<code>{duration, playbackRate, position}</code>).",


Yeah, I see what you mean here. IIRC, you had subfeatures for duration, playbackRate, position in your original PR (that data seems to have disappeared with a rebase, so I don't know this for a fact). Seeing this singular MediaPositionState feature though, without anything underneath, I don't think it adds anything to the parent feature—it's effectively saying, "this method has an argument, which is an object".

My inclination here is to drop this feature and move on.

Completely agree. Done!

Add FF for Android notes to match FF notes

hamishwillee · 2020-11-26T23:37:21Z

@ddbeck Thanks very much. Deleted the dictionary and accepted your note (yes, much better).

ddbeck

Finally! This was a hard one, but you stuck with it. Thank you, @hamishwillee! 🎉

hamishwillee · 2020-12-01T01:22:24Z

Thanks for your patience. Possibly the most important skill for a tech writer - even more than "writing good" :-)

github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 6, 2020

hamishwillee mentioned this pull request Oct 6, 2020

WebAPI updates for Firefox 82 mdn/sprints#3731

Closed

ddbeck added info needed 🔍 This needs more information to review or act on. not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. labels Oct 8, 2020

hamishwillee force-pushed the mediasession_always_on branch 2 times, most recently from 492d00d to d50b2dc Compare October 12, 2020 23:34

ddbeck self-requested a review October 15, 2020 18:55

ddbeck requested changes Nov 11, 2020

View reviewed changes

hamishwillee force-pushed the mediasession_always_on branch 3 times, most recently from 6cf6487 to 210f949 Compare November 13, 2020 01:06

hamishwillee commented Nov 13, 2020

View reviewed changes

ddbeck requested changes Nov 25, 2020

View reviewed changes

hamishwillee added 5 commits November 27, 2020 10:35

MediaSession enabled all builds from FF82

6f860b6

Add FF for Android notes to match FF notes

MediaPositionState dict as subfeature of setPositionState

df49e74

Update note and mark as partial implementation

af02bc3

Change note to match dbeck suggestion

26ea996

Remove MediaPostionState dictionary

d947b7d

hamishwillee force-pushed the mediasession_always_on branch from 6d1bc92 to d947b7d Compare November 26, 2020 23:35

foolip mentioned this pull request Nov 30, 2020

Add real values for Firefox for Media APIs #7522

Merged

ddbeck removed info needed 🔍 This needs more information to review or act on. not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. labels Nov 30, 2020

ddbeck approved these changes Nov 30, 2020

View reviewed changes

ddbeck merged commit ef9fc92 into mdn:master Nov 30, 2020

hamishwillee deleted the mediasession_always_on branch December 1, 2020 01:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MediaSession enabled all builds from FF82 #6845

MediaSession enabled all builds from FF82 #6845

hamishwillee commented Oct 6, 2020 •

edited

Loading

ddbeck commented Oct 8, 2020

hamishwillee commented Oct 8, 2020

hamishwillee commented Oct 8, 2020 •

edited

Loading

chrisdavidmills commented Oct 12, 2020

chrisdavidmills commented Oct 12, 2020

chrisdavidmills commented Oct 12, 2020

hamishwillee commented Oct 12, 2020 •

edited

Loading

hamishwillee commented Oct 12, 2020 •

edited

Loading

hamishwillee commented Oct 26, 2020

ddbeck commented Oct 26, 2020

hamishwillee commented Nov 8, 2020

ddbeck left a comment

ddbeck Nov 11, 2020

hamishwillee Nov 13, 2020 •

edited

Loading

ddbeck Nov 13, 2020

hamishwillee Nov 16, 2020 •

edited

Loading

hamishwillee Nov 16, 2020

ddbeck Nov 11, 2020

hamishwillee Nov 13, 2020

ddbeck Nov 13, 2020

hamishwillee Nov 16, 2020

ddbeck left a comment

ddbeck Nov 25, 2020

hamishwillee Nov 26, 2020

hamishwillee commented Nov 26, 2020

ddbeck left a comment

hamishwillee commented Dec 1, 2020

MediaSession enabled all builds from FF82 #6845

MediaSession enabled all builds from FF82 #6845

Conversation

hamishwillee commented Oct 6, 2020 • edited Loading

ddbeck commented Oct 8, 2020

hamishwillee commented Oct 8, 2020

hamishwillee commented Oct 8, 2020 • edited Loading

chrisdavidmills commented Oct 12, 2020

chrisdavidmills commented Oct 12, 2020

chrisdavidmills commented Oct 12, 2020

hamishwillee commented Oct 12, 2020 • edited Loading

hamishwillee commented Oct 12, 2020 • edited Loading

hamishwillee commented Oct 26, 2020

ddbeck commented Oct 26, 2020

hamishwillee commented Nov 8, 2020

ddbeck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamishwillee Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamishwillee Nov 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddbeck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamishwillee commented Nov 26, 2020

ddbeck left a comment

Choose a reason for hiding this comment

hamishwillee commented Dec 1, 2020

hamishwillee commented Oct 6, 2020 •

edited

Loading

hamishwillee commented Oct 8, 2020 •

edited

Loading

hamishwillee commented Oct 12, 2020 •

edited

Loading

hamishwillee commented Oct 12, 2020 •

edited

Loading

hamishwillee Nov 13, 2020 •

edited

Loading

hamishwillee Nov 16, 2020 •

edited

Loading