Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setPositionState: Explicit means to "clear" or reset. #315

Open
marcoscaceres opened this issue Jan 15, 2024 · 5 comments
Open

setPositionState: Explicit means to "clear" or reset. #315

marcoscaceres opened this issue Jan 15, 2024 · 5 comments
Assignees
Labels

Comments

@marcoscaceres
Copy link
Member

As part of #304, when calling setPositionState(state) we are having to do some WebIDL/prose gymnastics in order to "clear" the position.

In WebKit, there is support for passing null instead as a means of clearing the position state. It might be good to also add that in the spec to make clearing the position more ergonomic.

@marcoscaceres marcoscaceres changed the title Explicit means to "clear" or reset. setPositionState: Explicit means to "clear" or reset. Jan 15, 2024
@youennf
Copy link
Contributor

youennf commented Jan 18, 2024

In WebKit, there is support for passing null instead as a means of clearing the position state

I don't think this is valid to pass a null dictionary member as a method argument, WebKit implementation should probably be updated.

I think Chrome is supporting both setPostionState() and setPositionState({}) to clear the state.
But the latter throws in WebKit so maybe we could outlaw it.

If we agree on this, one possible WebIDL is:

dictionary MediaPositionState {
    required double duration; // make it required
    double playbackRate = 1; // add a default value
    double position = 0; // add a default value
}
partial interface MediaSession {
    undefined clearPositionState(); // needed?
    undefined setPositionState(); // clear state, same as above
    undefined setPositionState(MediaPositionState state);  // set state
}

@steimelchrome
Copy link
Contributor

Is there a reason WebKit needs to throw on setPositionState({})? The spec currently allows the website to send an empty MediaPositionState to clear it and only requires duration to be set for a non-empty MediaPositionState, which feels reasonable to me, and then the interface is just:

partial interface MediaSession {
  undefined setPositionState(optional MediaPositionState state = {});
}

@marcoscaceres
Copy link
Member Author

@steimelchrome, what you propose (with addition of @youennf's defaults) sounds quite reasonable to me and, as @youennf points out, it indeed does away with the need for clearPositionState().

We can update WebKit to match. Would you match it in Chrome?

@steimelchrome
Copy link
Contributor

I think the required on duration would prevent that from working, right? Should we remove that and then in the setPositionState() steps insist that duration must exist if the given position state is not empty?

@marcoscaceres
Copy link
Member Author

Yes, that would make sense, and it would be easy to enforce.

@marcoscaceres marcoscaceres self-assigned this May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants