Closed Bug 1827675 Opened 1 year ago Closed 9 months ago

`inverted-colors` media feature incorrectly matches smart invert

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Firefox 114
defect

Tracking

()

RESOLVED INVALID

People

(Reporter: lwarlow, Assigned: lwarlow)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

I tested the inverted-colors functionality in latest nightly and found that on macOS the implementation is spec in-compliant.

https://jsfiddle.net/4sg25tbe/4/

The background is red when macOS Invert Colours is off, or when it's on and it's set to classic mode. But blue when Smart Invert is enabled. This seems to be in direct contradiction of the specification https://www.w3.org/TR/mediaqueries-5/#valdef-media-inverted-colors-inverted which reads to me as if it says it should match the classic but not smart invert.

It's worth noting Safari's behaviour is also wrong https://bugs.webkit.org/show_bug.cgi?id=232041

I have researched ways to distinguish between Smart and Classic Invert on macOS to fix this bug in WebKit and have been unable to find a programmatic way to differentiate between them.

The severity field is not set for this bug.
:dholbert, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(dholbert)
Severity: -- → S3
Flags: needinfo?(dholbert)
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true

It's possible to tell the difference between classic and smart inverted colours using the com.Apple.Accessibility plist file -- try running
defaults read com.apple.Accessibility locally to see current vals on your system.

It looks like the relevant entries are:

  • AXSClassicInvertColorsPreference which is off when smart invert is selected and on when classic invert is selected. Disabling invert colours doesn't alter this preference.
  • InvertColorsEnabled which is on when invert colours is enabled and off when it is disabled

To read these, you probably want logic similar to what we have here for full keyboard access telemetry

Just as an update to this I confirmed the above is correct. My implementation is hooking into
AXSClassicInvertColorsPreference and it works a treat

To clarify that setting is 1 when invert is on and set to classic and 0 otherwise. You don't need to look at the other property at all.

Live update works by hooking into the existing AccessibilityDidChange notification used for other settings.

I can look into trying to patch this in gecko but might be easier if someone familiar with the codebase gave it a go.

Relevant code is here fwiw, if you wanna give it a go :)

Getting Firefox setup locally to give this a go.

Now correctly matches classic invert instead of smart invert.

Assignee: nobody → Luke
Status: NEW → ASSIGNED

Hopefully that patch has pushed up correctly and the code is correct. Give me a shout if you need me to make any changes. Have tested locally and this now matches Classic Invert and NOT smart invert on macOS which is expected behaviour (and matches my Chrome implementation).

I'm planning on doing a WebKit PR to address this issue there and then it should be interoperable across the board.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/610cf07a163c
Fix inverted-colors media query on macOS r=CanadaHonk,emilio
Blocks: 1846473

I may have misunderstood the Apple settings as an indicator when they're an instruction so perhaps it should match BOTH classic and smart invert? Or maybe it is correct to just match smart and not classic like Apple is doing. I'm unsure at this point so if someone else could sanity check to confirm that'd be useful. Happy to update the code accordingly if it should match BOTH or revert if it was correct the first time (I don't think it should match one and not the other)

I agree with this bug and earlier that it should match classic, and not smart. From spec (https://www.w3.org/TR/mediaqueries-5/#valdef-media-inverted-colors-inverted):

All pixels within the displayed area have been inverted. This value must not match if the user agent has done some kind of content aware inversion such as one that preserves the images (except through its UA style sheet, see below).

Afaik, classic invert matches this definition and smart does not.

My confusion comes from whether macOS actually does anything special when it comes to "Smart" invert. Or if the accessibilityDisplayShouldInvertColors is used to indicate to applications to implement smart mode (like the UA stylesheet)? The OS itself seems rather inconsistent as to whether anything happens.

Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch

Yes. The goal with “smart invert” is to allow devs to “double-invert” images (particularly photos) so they look “right” or “as they normally would” despite the OS doing a GPU-level pixel inversion. The API is available to devs when “smart invert” is on (so devs can respond) but not available when “classic invert” is selected (because devs sometimes do it wrong: double-inverting images of text for example)… e.g. “Just invert the whole screen, because the dev keeps undoing what I actually need here.”

Okay so based on the response in the WebKit slack I think I was wrong here and this change should be reverted. Sorry for the confusion and messing you guys around here.

[emilio, see comment 10 through 14; this might want a backout?]

Flags: needinfo?(emilio)
Flags: needinfo?(emilio) → needinfo?(sheriffs)

Backed out changeset 610cf07a163c (Bug 1827675) per emilio's request CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/386212864d07a46141ce5bca1ef4988cefa4bad7

Status: RESOLVED → REOPENED
Flags: needinfo?(sheriffs)
Resolution: FIXED → ---
Target Milestone: 118 Branch → ---
Status: REOPENED → RESOLVED
Closed: 9 months ago9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: