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

Control screen brightness 🔆 review feedback #335

Open
anssiko opened this issue Apr 21, 2022 · 42 comments
Open

Control screen brightness 🔆 review feedback #335

anssiko opened this issue Apr 21, 2022 · 42 comments

Comments

@anssiko
Copy link
Member

anssiko commented Apr 21, 2022

Mozillians have provided valuable feedback for the proposed new feature Control screen brightness 🔆: mozilla/standards-positions#623

Since Mozilla's standards-positions repository is not meant for API design discussions AFAICT, for simplicity, I'd propose we reuse this repo for that and loop in folks as appropriate.

Thanks @beaufortfrancois for kicking off this review round and @rakuco for preparing the explainer and everyone else who contributed to that. I'm wondering if @willmorgan would be interested in looking at Mozilla's feedback and reporting back here the findings?

@willmorgan
Copy link
Contributor

Yep, happy to look over any feedback Mozilla submit on the proposal. This is the only feedback I've seen so far, from @annevk.

Have you done an evaluation of how native platforms approach this? That might also help with figuring out how to address the open design issues.

Yes. On iOS and Android it's a synchronous call to the native API that sets brightness to a given value.

In general a request for this, similar to wake lock, seems okay. I'd be inclined to not give apps the result of such a request though and end users should always be able to override or automatically ignore such requests (ideally without the apps knowing).

At a minimum the use case I've submitted requires to know whether it fulfilled or not, but not necessarily the reason why the request was denied. An analogous example of this is the HTMLVideoElement.play() method throwing a deliberately vague NotAllowedError to avoid fingerprinting the device if, for example, video autoplay is disabled due to low battery or user preference.

@beaufortfrancois
Copy link
Contributor

@anssiko What are the next steps?

@anssiko
Copy link
Member Author

anssiko commented May 3, 2022

Proposed next steps for this issue:

  • Note Mozilla's review in Review requests
  • Ensure the explainer documents all the substantial review feedback received (Mozilla's and CSS WG's, others?), including any new open design issues or concerns
  • Document which proposed solution gained most support. If no clear winner emerged, that may suggest more iteration is needed.

Next steps for the effort as a whole:

  • Check we have volunteers to do the work (edit the spec, prototype in code). If positive, and if the feature is not just an extension to an existing Screen Wake Lock API surface, proceed to the spec incubation process, usually WICG. (Note: we can continue discuss the proposal in this WG while it is being incubated in WICG, but folks who want to do material contributions should join WICG too.)
  • When the incubation is completed successfully and there's huge excitement, some WG usually wants to adopt the work. This WG (DAS WG) is one possible home, and the expectation is the following:

The Working Group will not adopt new proposals until they have matured through the Web Platform Incubator Community Group or another similar incubation phase.

DAS WG's current technical scope contains "An API to prevent the screen from turning off" that we can argue contains a feature to "control screen brightness", so DAS WG adoption should be faster than some other WG.

I hope that helps explain both the immediate and more forward-looking path for this effort.

@willmorgan or @beaufortfrancois are in a good position to check the first three checkboxes with a PR and we'll take it from there.

Thanks for your contributions!

@willmorgan
Copy link
Contributor

@anssiko The above PR ☝️ #336 begins to take care of the first 3 points.

Happy to help with volunteering and excitement duties as well.

Should we put this to Chrome + WebKit teams? Sorry if obvious question, this is my first rodeo.

@anssiko
Copy link
Member Author

anssiko commented May 10, 2022

@anssiko The above PR ☝️ #336 begins to take care of the first 3 points.

Thanks! That ticked the first two checkboxes.

Current status: there seems to be a slight preference to a solution similar to wake lock outside this group.

Please chime in if you see something else.

Should we put this to Chrome + WebKit teams? Sorry if obvious question, this is my first rodeo.

Chrome: I guess @beaufortfrancois may be able to speak for Chrome?

WebKit: https://lists.webkit.org/pipermail/webkit-dev/2022-March/032160.html added to the explainer in #337

@beaufortfrancois
Copy link
Contributor

Chrome: I guess @beaufortfrancois may be able to speak for Chrome?

I'll let @reillyeon speak for Chrome.

@reillyeon
Copy link
Member

Chrome: I guess @beaufortfrancois may be able to speak for Chrome?

I'll let @reillyeon speak for Chrome.

I've reached out internally to check on resource availability and interest.

@anssiko
Copy link
Member Author

anssiko commented May 19, 2022

To keep folks on top, we're now at this step:

Document which proposed solution gained most support. If no clear winner emerged, that may suggest more iteration is needed.

As for the review requests, @marcoscaceres with his new hat may be equipped to get reactions to https://lists.webkit.org/pipermail/webkit-dev/2022-March/032160.html

@marcoscaceres
Copy link
Member

Thanks! Just acknowledging it as seen 👀. I'll try to get feedback to the WG - but if not, please continue. We were making really great progress.

Personally (not Apple/Webkit), as we discussed while putting together the document, I still think this is a "contrast" concern, not a "brightness" concern... though brightness certainly plays a role. We might want to reframe the document maybe around some kind of "optimal contrast" or something?... there is probably better wording.

Something to chew on a bit more.

@reillyeon
Copy link
Member

Discussing this proposal internally a question I'd like to see answered by the explainer is how implementations should ensure the user retains control over their screen brightness. With the Screen Wake Lock API the user can turn their screen off but a site can re-acquire the wake lock when the screen is turned back on again. If the same were allowed for screen brightness the user could get stuck in an unbreakable cycle of being blinded by their phone. We probably need something like the user gesture requirement to prevent that.

@beaufortfrancois
Copy link
Contributor

I'm adding a section dedicated to security in the explainer. Folks, please review at #338

@beaufortfrancois
Copy link
Contributor

@reillyeon The "security considerations" section has been added to the explainer: https://github.com/w3c/screen-wake-lock/blob/gh-pages/brightness-mode-explainer.md#security-considerations

@beaufortfrancois
Copy link
Contributor

beaufortfrancois commented May 27, 2022

By the way, I've been playing with one shape of the Screen Brightness API to experiment with locally and find potential issues.

You can find my WIP work (spec and Chromium CL) at https://github.com/beaufortfrancois/screen-brightness

@anssiko
Copy link
Member Author

anssiko commented May 30, 2022

@beaufortfrancois Your experimental implementation is hugely helpful in informing this incubation effort. I updated https://github.com/w3c/screen-wake-lock/blob/gh-pages/brightness-mode-explainer.md#screen-idl-interface-extension so folks find your work from the explainer.

@beaufortfrancois
Copy link
Contributor

Thank you @anssiko!

@anssiko
Copy link
Member Author

anssiko commented May 30, 2022

My pleasure.

@jwheare
Copy link

jwheare commented Jun 10, 2022

Given this would require an explicit user gesture, there would presumably need to be some text to prompt the user to increase brightness. From a UX perspective it would feel bad to display this prompt if the brightness is already at max.

I understand the concerns around fingerprinting re: no ability to read the device brightness, but a simple boolean for isMaxBrightness would be helpful to avoid unnecessary user prompts while mitigating high resolution fingerprinting attempts.

@eligrey
Copy link
Member

eligrey commented Jun 10, 2022

Web content should be able to control its own brightness without needing a user gesture. In fact, this is already allowed with HDR videos. If the user gesture is meant to protect against harms from dangerous levels of perceived contrast then the physical threat model is already broken.

Existing "normal" levels of brightness can already be abused to trigger epileptic seizures in sensitive individuals.

We need active device-level interventions against perceptually harmful contrast yesterday, and it's a separate issue from letting sites control their own brightness.

Policing abuse of screen brightness should not be under the purview of browsers. That is a matter best handled through device-level drivers. For example, devices could actively monitor ambient brightness and limit potentially harmful levels of contrast at a system level.

@willmorgan
Copy link
Contributor

Web content should be able to control its own brightness without needing a user gesture. In fact, this is already allowed with HDR videos. If the user gesture is meant to protect against harms from dangerous levels of perceived contrast then the physical threat model is already broken.

Existing "normal" levels of brightness can already be abused to trigger epileptic seizures in sensitive individuals.

We need active device-level interventions against perceptually harmful contrast yesterday, and it's a separate issue from letting sites control their own brightness.

My basis for suggesting that the API is gated by user gesture (and feature policy) is to prevent third party origins and scripts from increasing screen brightness without user consent to prevent against battery drainage and ad annoyances.

@eligrey
Copy link
Member

eligrey commented Jun 10, 2022

@willmorgan Third party origins can already do this with HDR videos on certain displays. Are you suggesting that we should gate all luminance control in browsers to require user gestures?

@willmorgan
Copy link
Contributor

@willmorgan Third party origins can already do this with HDR videos on certain displays. Are you suggesting that we should gate all HDR luminance control in browsers to require user gestures?

This proposal doesn't aim to deal with HDR videos, but I would have thought that the existing restrictions on HTML media controls would at least partly prevent that abuse.

@eligrey
Copy link
Member

eligrey commented Jun 13, 2022

I'm not saying that this proposal needs to be changed to be video-centric, I'm saying that luminance control capabilities already exist in browsers under a less-than-ideal API that is not gated by user gestures.

@willmorgan
Copy link
Contributor

Alright @eligrey, thanks for clarifying. Do you have any suggestions for an ideal API?

@eligrey
Copy link
Member

eligrey commented Jun 13, 2022

Maybe a CSS luminance property that takes nits as a value? Vague unit-less 'brighter' adjectives don't seem as useful for aesthetical purposes (which imo is one of the only valid use cases).

@beaufortfrancois
Copy link
Contributor

Thank you @eligrey for your suggestions. We had thought about CSS at https://github.com/w3c/screen-wake-lock/blob/gh-pages/brightness-mode-explainer.md#css-property

Note that a CSS WG folk suggested a JS approach: w3c/csswg-drafts#6990 (comment)

Looking over this, I think your JS API looks reasonable. This is something that should be permissioned, so I don't think we want this to be controllable via a CSS property. (In any case, this is more document-global rather than element-specific.)

@eligrey
Copy link
Member

eligrey commented Jun 13, 2022

That CSS WG critique seems to be unaware that this is an existing unpermissioned feature already present for rendering HDR videos.

My opinion is that perceptually harmful contrast mitigation is entirely out of scope for the web. That is best left to the domain of device drivers and OS frameworks.

After invalidating that critique, the logical conclusion is that CSS is likely the best abstraction here as this should be permissionless

@willmorgan
Copy link
Contributor

@eligrey I took a deeper look into HDR capabilities and I'm not sure they would solve my particular use case, because there'd actually be very little dynamic range as the use case flashes a single colour across the screen rather than a set of colours in different regions.

If we are taking an anonymous "aesthetics only" use case for brightness reasons then would it not make sense to have some indication whether or not that level of brightness was achieved? If so, then a JS API, where you can inspect capabilities and tell if a request is successful or not, makes sense IMHO.

I still think that user consent is needed to increase the brightness of the screen to prevent draining the battery and prevent user annoyance, and don't believe there's a way to prevent this in CSS.

@eligrey
Copy link
Member

eligrey commented Jun 13, 2022

While it may be useful to read achievable luminance values via getComputedStyles or be able to evaluate media queries that target specific screen brightness capabilities, do note that this would be yet another fingerprinting vector for users and a way for ads to target users with more expensive displays.

If you're worried about battery drain, then you should also be arguing for HDR video rendering to require explicit user permission (which is unreasonable IMO). Brightness limitations for battery savings belongs in OS level frameworks, not browser permission systems.

@willmorgan
Copy link
Contributor

willmorgan commented Jun 16, 2022

Given this would require an explicit user gesture, there would presumably need to be some text to prompt the user to increase brightness. From a UX perspective it would feel bad to display this prompt if the brightness is already at max.

I understand the concerns around fingerprinting re: no ability to read the device brightness, but a simple boolean for isMaxBrightness would be helpful to avoid unnecessary user prompts while mitigating high resolution fingerprinting attempts.

Would permission prompt text suffice for advisory text? As an aside in my experience it makes sense to explain to the user what's happening and why in addition to any browser based prompts.

Having access to isMaxBrightness would be useful - if this is permission gated (via user gesture or feature policy) then setting that value sounds like it wouldn't unduly increase fingerprint surface.

@beaufortfrancois
Copy link
Contributor

That CSS WG critique seems to be unaware that this is an existing unpermissioned feature already present for rendering HDR videos.

My opinion is that perceptually harmful contrast mitigation is entirely out of scope for the web. That is best left to the domain of device drivers and OS frameworks.

After invalidating that critique, the logical conclusion is that CSS is likely the best abstraction here as this should be permissionless

Not all device screens have a way to increase the brightness in certain parts of the screen (like HDR nowadays).

I still think increasing the overall brightness of the device screen while a website is visible to the user is valuable in cases we've enumerated in the explainer. Even though permission-less is technically possible today with hacks, it is not 100% reliable as it depends on specific hardware.
A standardised Screen Brightness API on the other hand would allow websites to request for brightness increase in a consistent way that could be granted when certain conditions are met.

@beaufortfrancois
Copy link
Contributor

I'll note that the initial tweet got 21 Retweets and 77 Likes ❤️ . This definitely counts as an encouraging and positive signal from web developers.

@anssiko
Copy link
Member Author

anssiko commented Jun 27, 2022

I'll note that the initial tweet got 21 Retweets and 77 Likes ❤️ . This definitely counts as an encouraging and positive signal from web developers.

Noted this substantial ❤️ from web developers in 57d993f -- the more signals the better.

@willmorgan
Copy link
Contributor

Even though permission-less is technically possible today with hacks, it is not 100% reliable as it depends on specific hardware.

I experimented with this, and it only works if you're looking to display a single colour. For videos and images the hack displays a massively oversaturated image, which isn't a good end goal! 😁

@willmorgan
Copy link
Contributor

willmorgan commented Jun 29, 2022

This can be raised at https://github.com/WebKit/standards-positions

(Note to self for later when not on mobile on a slow train)

@marcoscaceres
Copy link
Member

yep, and add me as the "webkitten" 😸

@willmorgan
Copy link
Contributor

@Genbuchan I've noticed your feedback and reactions on various posts including the new WebKit standards positions request.

Would you mind elaborating on your position a little bit?

@Genbuchan
Copy link

@Genbuchan I've noticed your feedback and reactions on various posts including the new WebKit standards positions request.

Would you mind elaborating on your position a little bit?

In the first I don't think Screen Wake Lock API is good. Because this API spec doesn't seem consider about non-LCD display.

Non-LCD display (e.g., CRT and OLED) can cause screen burn if the brighter pixels are continually lit. However it is technically difficult for browsers and OS to identify the type of display, making it difficult to resolve the problem. Control screen brightness proposal probably exacerbate the problem...

@beaufortfrancois
Copy link
Contributor

Non-LCD display (e.g., CRT and OLED) can cause screen burn if the brighter pixels are continually lit. However it is technically difficult for browsers and OS to identify the type of display, making it difficult to resolve the problem.

Thank for your answer @Genbuchan!

Browsers could mitigate this issue by showing a persistent notification when a web app increases the screen brightness so that the user can easily reset it in one click. Rejecting all requests on desktop is also a possibility for a user agent if they're not confident about the type of display.

@tomayac
Copy link
Contributor

tomayac commented Jul 4, 2022

Native apps also have this ability and I personally haven’t seen any app abuse this power, since bumping to maximum screen brightness is very noticeable. I’ve only ever seen it with a user gesture, for example when tapping a QR code.

@eligrey
Copy link
Member

eligrey commented Jul 7, 2022

Non-LCD display (e.g., CRT and OLED) can cause screen burn if the brighter pixels are continually lit. However it is technically difficult for browsers and OS to identify the type of display, making it difficult to resolve the problem.

This is already handled by the firmware in current displays. Some older OLED displays (and most CRT displays) don't have this functionality, but modern OLED displays will automatically dim and pixel-shift to prevent burn in when the display detects that it is being driven at a high brightness for too long ('too long' is determined by thermal design constraints). Similarly, modern LCD-based displays also dim their backlights upon reaching thermal constraints to prevent backlight degradation.

Again, I don't think brightness control should be gated by a permission, but I do think it should be scoped to web-controlled render surfaces (i.e. we shouldn't expose system-level screen brightness control to the web at all). Unfortunately this necessitates an implementation exclusive to HDR-supporting devices. Fortunately "HDR-supporting devices" includes most OLED phones newer than the Galaxy S5 from 8 years ago.

@beaufortfrancois
Copy link
Contributor

Following up on #335 (comment) and WebKit/standards-positions#19 (comment), I've been playing with yet another shape of the Screen Brightness API to experiment with locally and find potential issues.

This time, it's the "requestFullscreen() integration". You can follow my WIP at https://github.com/beaufortfrancois/screen-brightness

@willmorgan
Copy link
Contributor

I summarised the discussion so far in this little mindmap. You might need to open the image in a new tab to see it in full.

image

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

No branches or pull requests

9 participants