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

Current API not friendly to components #55

Closed
sicking opened this issue Jul 14, 2015 · 7 comments
Closed

Current API not friendly to components #55

sicking opened this issue Jul 14, 2015 · 7 comments

Comments

@sicking
Copy link

sicking commented Jul 14, 2015

My impression is that the main consumer of the API is video playing websites. I.e. holding the screen awake while a video is playing.

However more often than not video playing is done through a library. Either in order to provide fallback to flash, or in order to provide a customized player UI, or both.

However the current API doesn't seem very friendly to such a library. The library could simply set keepScreenAwake=true when playing starts, and keepScreenAwake=false when playing ends. However that might result in the lock getting released too early if something else on the page also wants to hold the lock alive. For example if another video is playing.

This, or at least a similar, concern was brought up earlier against the spec. At that time the response was that the library could be written such that it takes a callback, and then calls that callback in order to grab/release a screen lock. The main page could then take care of coordination if multiple components on the page attempts to grab a lock at once.

This doesn't seem like a particularly good solution though. It means that API isn't a good fit for what is likely the main use case of this API, video-playing through a library. While a workaround is possible, it is fairly awkward and clearly a workaround.

Concretely, it would likely either mean that adoption of the API is significantly slowed down since it requires both video playing libraries and webpages using those libraries to adopt the API. But more likely is that pages will simply ignore the problem of multiple components grabbing the lock at once, leading to buggy pages where the lock gets released too early, which will result in a bad experience for users.

Instead I suggest we adopt an API like

screen.keepAwakeUntil(promise);

(Feel free to bikeshed the name, or put it on a different object)

which would keep the screen kept awake until the promise is resolved. This function could be called any number of times and only once the last promise is resolved is the screen lock released. This way the video player library can release its screen lock without worrying about bad interactions with other components on the same page, and without having to change its API towards the embedding page.

This approach also has the advantage of tying into promise infrastructure, which likely will mean that many times the page doesn't have to remember to explicitly release the lock, but can instead just grab an existing promise.

@annevk
Copy link
Member

annevk commented Jul 15, 2015

That makes sense to me. That was also proposed for the "Busy API" in this thread: whatwg/fetch#19

@marcoscaceres
Copy link
Member

I also agree that this approach is better.

@andrey-logvinov
Copy link
Collaborator

Good point about the components. Is promise the right solution though? Fundamentally, a promise has two terminal states, resolved and rejected. Should only the former cause a wake lock request from the component to be cancelled, or both?
Also, the promise could be already resolved or rejected at the time of request, but I believe in this case the request should just be ignored (as the "until" event has already happened).

@sicking
Copy link
Author

sicking commented Jan 23, 2016

The lock would be released on both resolve and reject.

@andrey-logvinov
Copy link
Collaborator

@sicking Wouldn't it make more sense to keep the lock flag as an attribute but apply it at a finer granularity level, e.g. document.getElementById("myid").keepScreenAwake = true; vs Screen.keepAwake = true;? It would solve the problem with components and also make it easy to tie lock applicability to element visibility. For example, Android provides this functionality for views: http://developer.android.com/reference/android/R.attr.html#keepScreenOn

Are there any considerations that make the promise approach preferable to the element attribute approach?

@sicking
Copy link
Author

sicking commented Feb 18, 2016

That could also work yes.

I don't have any strong feelings, but having a standalone API seems more in line with the extensible web manifesto than adding API surface to elements.

@andrey-logvinov
Copy link
Collaborator

The new version of the spec allows multiple independent requests on the same page. Please check the updated spec and if there are still problems, create new issues for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants