Skip to content

Add missing preference info to playbackState#6465

Merged
ddbeck merged 1 commit intomdn:masterfrom
a2sheppy:playbackState-add-pref
Aug 19, 2020
Merged

Add missing preference info to playbackState#6465
ddbeck merged 1 commit intomdn:masterfrom
a2sheppy:playbackState-add-pref

Conversation

@a2sheppy
Copy link
Contributor

@a2sheppy a2sheppy commented Aug 3, 2020

This follows up on my initial PR for playbackState in
Firefox 80 by adding the missing information that
you have to enable the preference to make it work.
This is global across the entire API (kind of makes
you wish you could do this in just one place), and
I previously missed adding it here.

This follows up on my initial PR for playbackState in
Firefox 80 by adding the missing information that
you have to enable the preference to make it work.
This is global across the entire API (kind of makes
you wish you could do this in just one place), and
I previously missed adding it here.
@a2sheppy a2sheppy requested a review from ddbeck August 3, 2020 16:05
@ghost ghost added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Aug 3, 2020
@ddbeck
Copy link
Contributor

ddbeck commented Aug 3, 2020

No, you're right: you should be able to do it in one place. Since the parent feature has the flag, you don't need to add one here. Really, the metadata is wrong: it shouldn't have the flag data, since it's the same flag as the parent feature.

@a2sheppy
Copy link
Contributor Author

a2sheppy commented Aug 3, 2020

No, you're right: you should be able to do it in one place. Since the parent feature has the flag, you don't need to add one here. Really, the metadata is wrong: it shouldn't have the flag data, since it's the same flag as the parent feature.

Does that actually work? I was under the impression that you had to include it on every item for it to show up on each page in the interface properly.

@ddbeck
Copy link
Contributor

ddbeck commented Aug 4, 2020

Hmm, maybe this is an odd one. I'd appreciate @Elchi3's take here.

Really, we need a guideline about this. I was under the impression that if a parent feature was gated behind a preference that we didn't need the preference on the subfeatures. Maybe it's a bit confusing in this case. Is it plausible that you'd use MediaSession.playBackState without needing the parent or related features? Does api.Navigator.mediaSession need a preference (the data says no, but I'm skeptical)? Is it possible to even get a MediaSession object without turning on the flag?

To actually answer your question, it works in the sense that the data is valid and makes sense. And it's easy to maintain: when MediaSession comes out from behind the flag, you could show support for the feature api.MediaSession without touching all its descendants (assuming that's how it actually works).

A fake example: if you've got SomeObject (behind a flag), SomeObject(), and SomeObject().someMethod(), you'd necessarily hit ReferenceError: SomeObject is not defined before you'd hit an error about someMethod() or the constructor. In other words, you don't need a preference to turn on someMethod, you need the preference to turn on SomeObject (you get all the descendants included). When the flag is no longer needed, we don't have to change 3+ features, just one.

In terms of showing that on every page of an interface on MDN, I'd characterize this as yet another consumer-side issue, a problem with the way we generate tables.

@Elchi3
Copy link
Member

Elchi3 commented Aug 5, 2020

When displaying the data on MDN, we don't automatically inherit anything to the sub features, so flag information isn't inherited here either. We can discuss if we should implement such inheritance and what it does exactly.

For BCD, think we added flags throughout, so tables look sensible for main features and for sub features. This practice is very MDN-rendering-driven, I'm not sure if it is the best for other data consumers. It seems we have no clear guideline, so that's a bug for the data model.

@a2sheppy
Copy link
Contributor Author

a2sheppy commented Aug 5, 2020

So that means that what I've done here doesn't need changing after all. Thanks, @Elchi3. I do think we should look into the possibility of redefining things so that for members of an interface, the experimental, standards_track, and deprecated flags under status to allow a special value of inherit to allow the inheriting of the interface's state. Same thing for the version block's flags object; it should allow inherit to inherit the parent's flags, if any. That would both clarify the state of these things and allow a substantial reduction in editing complexity in many cases, I think. As well as reducing the JSON size in a lot of cases, too.

I thought about just allowing these to be left out to indicate "inherit" but that introduces ambiguity with the flags object in particular.

@a2sheppy
Copy link
Contributor Author

a2sheppy commented Aug 5, 2020

Anyway, @ddbeck, can you review when you have time? Thanks! :)

@ddbeck
Copy link
Contributor

ddbeck commented Aug 10, 2020

I think we need to actually specify what we're doing here with flag data—the discussion here has made reviewing flag data a bit fraught. To that end, I've opened #6509.

@ddbeck ddbeck removed their request for review August 10, 2020 14:49
Copy link
Contributor

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the situation at Mozilla, I don't see #6509 being resolved more robustly anytime soon, so I'm going to accepted this as-is. Thanks, Sheppy!

@ddbeck ddbeck merged commit 99f7087 into mdn:master Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants