Add security considerations section #338
Conversation
|
@beaufortfrancois thanks much! @rakuco PTAL |
Done. Thanks for catching! |
|
|
||
| - The screen brightness can be controlled only in response to a user gesture to ensure user retain control over their screen brightness. This prevents a situation where a site increases screen brightness every time the system or user overrides it manually. | ||
|
|
||
| - If the page visibility becomes hidden after screen brightness has been increased, the screen brightness should be restored automatically. |
There was a problem hiding this comment.
I would also add that if the brightness change request is denied, it could/should be denied with a generic reason where it isn't possible to necessarily distinguish why the request failed.
There was a problem hiding this comment.
Thank you @willmorgan I've actually went further. See my latest patch. What do you think?
There was a problem hiding this comment.
It would be helpful to know whether the request succeeded or not, but not necessarily why. So we would blanket deny with a NotAllowedError and not go any more granular than that.
There was a problem hiding this comment.
What would be the requirements to know whether the request succeeded or not?
Thinking out loud: If the screen brightness is already at its maximum and you call await screen.requestBrightnessIncrease() would you expect this to fail even it didn't succeed?
I think having this baked in would help adoption of this feature from a security/privacy perspective.
There was a problem hiding this comment.
Good question. I think if there's no gesture, battery level is too low, or feature policy is misconfigured (assuming we're doing feature policy!), I'd expect to see something like a NotAllowedError.
If the brightness is already at 100% and we call await screen.requestBrightnessIncrease(), then I'd expect the promise to fulfil. Otherwise, it might be possible to infer the brightness of the screen. If we allowed that to happen it would contradict the intention to prevent reading the screen brightness - reading the screen brightness is a non-goal of this API for privacy reasons anyway.
There was a problem hiding this comment.
@willmorgan Thank you! I've updated the explainer with your feedback. PTAL
|
Please have another look with latest changes. |
There was a problem hiding this comment.
Looking good! I'm not sure if we should consider here any interplay with the multi screen / window placement API, as that may be relevant when it comes to releasing the request. I think it might be better to address those points in more detail when there is an agreed API surface choice. What do you think @anssiko?
|
@willmorgan I think that is worth noting in the explainer. And latest when the API is more fleshed out any hooks that may be needed for https://github.com/w3c/window-placement should be brought to the attention of folks working on that API by opening a new issue for a future enhancement. |
|
@anssiko Shall we merge? What are the next steps? |
|
Thanks @beaufortfrancois @willmorgan. With two reviews I think we’re good to merge this. I’m sure @rakuco is happy too and if he has further refinements we’ll get them in as we iterate. As for the next steps for the proposal overall, we’re at #335 (comment) |
To address concerns raised by @reillyeon at #335 (comment), I'm adding a security considerations section to the screen brightness explainer.
@anssiko Please have a look