Page MenuHomePhabricator

Using skin-invert on a image (thumb) makes the figcaption unreadable in dark mode
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Assigned To
Authored By
Lofhi
May 16 2024, 8:35 AM
Referenced Files
F55319668: 2024-06-14_10-48-08.png
Jun 14 2024, 5:49 PM
F55319595: 2024-06-14_10-45-59.png
Jun 14 2024, 5:47 PM
F55319479: 2024-06-14_10-41-07.png
Jun 14 2024, 5:47 PM
F55093287: screenshot 602.png
Jun 9 2024, 2:39 AM
F53974291: image.png
May 20 2024, 5:34 PM
F53675680: image.png
May 18 2024, 6:49 AM
F53675281: image.png
May 18 2024, 6:44 AM
F53668230: image.png
May 18 2024, 5:21 AM

Description

Steps to replicate the issue (include links if applicable):

What happens?:
A figcaption of an image using the skin-invert CSS class is also inverted.

image.png (409×509 px, 46 KB)
image.png (412×505 px, 54 KB)
Without skin-invert CSS classWith skin-invert CSS class

What should have happened instead?:
Only the image should be inverted.

Acceptance criteria

  • A new skin-invert-image class is added that allows editors to mark up HTML so that in dark mode, img tags found within the element are inverted. e.g. .skin-invert-svg img { filter: invert( 1 ); }
  • If editors, mix skin-invert and skin-invert-svg, this will be considered either bad input from the editor or is used intentionally and we don't need to worry about this case.
  • Class should apply to Minerva and Vector 2022 skins.

QA

The image should show on https://en.wikipedia.beta.wmflabs.org/wiki/T365102?vectornightmode=1

Requirement

Add a new skin-invert-image class to allow editors to mark up HTML so that in dark mode, only the img tags within the element are inverted. Ensure this class applies to both Minerva and Vector 2022 skins. Verify the implementation on the specified beta test page and the provided production page.

BDD

gherkin
Feature: Apply skin-invert-image Class for Image Inversion in Dark Mode

  Scenario: Invert only img tags within elements using skin-invert-image class
    Given an image element using the skin-invert-image class
    When the user views the image in dark mode
    Then image shows with correct contrast
    And this should apply to both Minerva and Vector 2022 skins

Test Steps

Test Case 1: Verify skin-invert-image Class Implementation

  1. Enable dark mode in the Vector 2022 skin.
  2. Navigate to the test page on beta or production.
  3. Verify that only the img tag within the element using the skin-invert-image class is inverted.
  4. AC1: Image shows with correct contrast

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia): WMF-hosted frwiki

Other information (browser name/version, screenshots, etc.):

Sign off

https://www.mediawiki.org/wiki/Recommendations_for_night_mode_compatibility_on_Wikimedia_wikis is updated to reference the new class.

QA Results - FAIL

ACStatusDetails
1T365102#9894024 failure is addressed in T367589

Event Timeline

Lofhi renamed this task from Using skin-invert on a image (thumb) makes the figcaption unreadable to Using skin-invert on a image (thumb) makes the figcaption unreadable in dark mode.May 16 2024, 9:09 AM

I guess we need a specific class for inverting thumbnails @Jdrewniak ?

This doesn't seem to be a solution. You now have two different displays for thumbs. And I really don't see how you want editors to adopt the new practice of adding two CSS classes (they usually don't even know what CSS is ; neither use |class= on thumbs for the last twenty years ; not to mention the fact that some major contributors will probably never activate the dark mode and take this problem into account) on every article where thumbs need it without digging through lost project internal wiki documentation.

Fixing thumbs ≠ having specific thumbs being styled across themes in exactly the same way (and differently from the rest of the thumbs) without consideration of user theme choice. We have a solution for simple images, we should have one for thumbs.

There are even inconsistencies using .skin-invert between the figure background and the figcaption backgrounds. I think that the filter should be only on the image element. Or, the .skin-invert class seems to add the filter... and switch to color-scheme: light.

If we touch the limits of the .skin-invert class, I think that a specific class for inverting thumbnails is a good solution if it avoids this weird combo of classes.

image.png (523×1 px, 123 KB)
image.png (303×296 px, 31 KB)
Two different appearances for the same component on the same article.What we should aim for.

Also, per this diff, it seems that you are advising that the communities should never use a white background (meaning using the Codex token preferably).

Or, in a very theoretical case, some images should perhaps not have inverted colors, but be unreadable with a dark background. In such cases, perhaps a second class should exist to add a white background to the image. Exemple (but here it's better to invert):

image.png (297×287 px, 29 KB)

This class doesn't exist today, and this is what's likely to cause the addition in future of inline style rules that will add technical debt, such as background-color: white in thumbs.

Okay, opening to reconsider a class to mean "this SVG needs a white background in dark mode"

A class to mean "this image needs a white background in dark mode" is appreciable if it's WMF handcrafted (could be useful in some years if you develop a new theme, etc.), but I maintain that some images don't need a white background, just a invert & rotation filter (as skin-invert) that don't affect the thumb, only the image element.

Also, if the WMF wants this issue contrast issue to be handled by regular editors and not only wikignomes, the Visual Editor should be improved and I think we shouldn't use |class= thumb option, but specific new format options for these two new cases (|i18nwhitebackground).

Example:

image.png (833×600 px, 42 KB)

Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)

@Jdlrobson: I don't understand why the class should only affect SVG. Can you explain me your choice?

It doesn't only impact SVG. The rule .skin-invert-svg img would apply to all IMG tags, it's just a proposed class name (and SVGs would be the main benefactors as PNGs do not always invert very well).

Change #1038903 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Add skin-invert-svg class

https://gerrit.wikimedia.org/r/1038903

Change #1039781 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/MinervaNeue@master] Add skin-invert-image class

https://gerrit.wikimedia.org/r/1039781

Change #1038903 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Add skin-invert-image class

https://gerrit.wikimedia.org/r/1038903

Change #1039781 merged by Jdlrobson:

[mediawiki/skins/MinervaNeue@master] Add skin-invert-image class

https://gerrit.wikimedia.org/r/1039781

Edtadros subscribed.

Test Result - Beta

Status: ✅ PASS
Environment: Beta
OS: macOS Sonoma
Browser: Chrome
Device: MBA
Emulated Device: NA

Test Artifact(s):

Test Steps

Test Case 1: Verify skin-invert-image Class Implementation

  1. Enable dark mode in the Vector 2022 skin.
  2. Navigate to the test page on beta or production.
  3. Verify that only the img tag within the element using the skin-invert-image class is inverted.
  4. AC1: Image shows with correct contrast

screenshot 602.png (902×1 px, 205 KB)

All done, resolving

@Jdlrobson @Edtadros The prod link listed didn't go anywhere. I tried it in test.wiki as seen in the screenshot below but it didn't work. I also went to the PROD link of Gas flare where the original image is from and it's fine. Is there another way to test this or is the prod link I provided with a screenshot suffice for testing?

Test Result - PROD

Status:❌ FAIL
Environment: PROD
OS: macOS Sonoma 14.5
Browser: Chrome 126
Device: MBA
Emulated Device: NA

Test Artifact(s):

Test Steps

Test Case 1: Verify skin-invert-image Class Implementation

  1. Enable dark mode in the Vector 2022 skin.
  2. Navigate to the test page on beta or production.
  3. Verify that only the img tag within the element using the skin-invert-image class is inverted.
  4. AC1: Image shows with correct contrast

https://test.wikipedia.org/wiki/File:Gas_flare_fr.svg

2024-06-14_10-41-07.png (822×2 px, 218 KB)

https://en.wikipedia.org/wiki/Gas_flare

2024-06-14_10-45-59.png (1×3 px, 661 KB)

https://fr.wikipedia.org/wiki/Torchage_du_gaz_naturel?vectornightmode=1

2024-06-14_10-48-08.png (1×2 px, 759 KB)

https://fr.wikipedia.org/wiki/Torchage_du_gaz_naturel?vectornightmode=1 was the only URL to test here and it looks like this hasn't worked to spec. I've created T367589 to follow up here. Thanks for testing!