1827675 - `inverted-colors` media feature incorrectly matches smart invert

Status: RESOLVED INVALID

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

Description

[Luke Warlow]

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.

Comment 1

[Luke Warlow]

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.

Comment 2

[BugBot [:suhaib / :marco]]

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

For more information, please visit auto_nag documentation.

Comment 3

[Morgan Reschenberg [:morgan]]

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

Comment 4

[Luke Warlow]

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.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

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

Comment 6

[Luke Warlow]

Getting Firefox setup locally to give this a go.

Comment 7

[Luke Warlow]

Attachment: Bug 1827675 - Fix inverted-colors media query on macOS

Now correctly matches classic invert instead of smart invert.

Comment 8

[Luke Warlow]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/610cf07a163c
Fix inverted-colors media query on macOS r=CanadaHonk,emilio

Comment 10

[Luke Warlow]

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)

Comment 11

[Oliver Medhurst [:canadahonk]]

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.

Comment 12

[Luke Warlow]

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.

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/610cf07a163c

Comment 14

[Luke Warlow]

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.

Comment 15

[Daniel Holbert [:dholbert]]

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

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Could we back out https://hg.mozilla.org/mozilla-central/rev/610cf07a163c please?

Comment 17

[Noemi Erli[:noemi_erli]]

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

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

Comment 18

[Sandor Molnar[:smolnar]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/386212864d07