html.skin-theme-clientpref-night .mw-parser-output figure img

Please refine this selector so that it catches the specific figures you intend to catch. This would catch images inside figcaption and would have knockon effects at some point for having access to wikitext figure.

(Haven't really done a good look at this otherwise.)

Note that it would also need to catch images in infoboxes like in Horsepower

On second thought, we should probably avoid infoboxes because many of those already have inverts applied to them e.g. signatures.

In T370074#10000361, @Izno wrote:

html.skin-theme-clientpref-night .mw-parser-output figure img

Please refine this selector so that it catches the specific figures you intend to catch. This would catch images inside figcaption and would have knockon effects at some point for having access to wikitext figure.

(Haven't really done a good look at this otherwise.)

... and looking at it, this should ignore already-applied skin-invert additions. As suggested above. :)

After looking at it, I think a preferable selector would be closer to

html.skin-theme-clientpref-night figure[typeof~="mw:File"]:not(.skin-invert) > a > img

Pretty certain it does not need to be restricted to .mw-parser-output, as 1) the specificity is getting high here, and 2) the addition of the typeof attribute selector reasonably takes its place since that only occurs where parser output is.

If you're trying to communicate that it can't be embedded in something else, this is going to be a problem when <section>s roll out in Parsoid read view. And also the signatures in tables problem above.

Also added the child > a > to avoid figcaption overrunning. Not sure there's another way to do that nicely, and there are images which do not have links.

@Izno we recently changed the implementation proposal to add ">" between mw-parser-output and figure to avoid it applying to templates. I think that is preferable to a selector that applies to all figure elements having observed how this is working in production and the bug reports we've been getting.

The not is only necessary inside templates. If skin-invert is applied to a thumbnail with this background it will work the same. It will actually be more readable. Take a look at "Use of diaeresis" on https://en.wikipedia.org/wiki/Greek_alphabet for example.

In T370074#10012256, @Jdlrobson wrote:

@Izno we recently changed the implementation proposal to add ">" between mw-parser-output and figure to avoid it applying to templates. I think that is preferable to a selector that applies to all figure elements having observed how this is working in production and the bug reports we've been getting.

.mw-parser-output > figure will miss images in mobile domain (today), Discussion Tools (soon), and Parsoid read views (later) because mw-parser-output is not their direct parent. Which I think is a pretty significant portion of the views you're trying to correct here. It sounds like you may need to check for other uses of .mw-parser-output >...

I would guess there's also a series of tables with chemicals and signatures that you will miss? But maybe those are ad hoc enough?...

That aside, I'd still like to see figure[typeof~="mw:File"] per previous comment, and it would still be good to avoid any images in figcaption which I'm still not sure of the best way :').

The not is only necessary inside templates. If skin-invert is applied to a thumbnail with this background it will work the same. It will actually be more readable. Take a look at "Use of diaeresis" on https://en.wikipedia.org/wiki/Greek_alphabet for example.

I'm not sure I'm following this comment though. Not a big deal though. I'd just recommend for this having a comment in the patch that makes clear the target of the rule is "top level" images outside of templates.

Thanks for the feedback and refinement @Izno, I appreciate it! Updated the description to take all of this into account.

I would guess there's also a series of tables with chemicals and signatures that you will miss

Yes, for now, I think it's better to fix these on the template level and avoid general solutions.

Change #1058269 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Skins: Apply a background color to images in dark mode

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

Change #1035817 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] WIP: Clean up Minerva image styles

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

Change #1058619 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Dark mode: Images should have background

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

Change #1058269 abandoned by Jdlrobson:

[mediawiki/core@master] Skins: Apply a background color to images in dark mode

Reason:

I've decided to do this in Vector 2022 for now: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/1058619?usp=search

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

Change #1058643 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] File pages: Apply background in dark mode to file pages

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

Hey @JScherer-WMF please see T371529#10033077 - DST are concerned that we are using an inappropriate token for the background here.

In T370074#10033615, @Jdlrobson wrote:

Hey @JScherer-WMF please see T371529#10033077 - DST are concerned that we are using an inappropriate token for the background here.

I'm fine to wait for a new token for this and mark this as blocked on others in the meantime. @CCiufo-WMF and DST can decide what naming convention makes the most semantic sense for dark mode image backgrounds. If we decide we don't want to wait, we can use the hex described in the task description, mark it as tech debt, and update it to the design token when it's available.

In T370074#10033801, @JScherer-WMF wrote:

In T370074#10033615, @Jdlrobson wrote:

Hey @JScherer-WMF please see T371529#10033077 - DST are concerned that we are using an inappropriate token for the background here.

I'm fine to wait for a new token for this and mark this as blocked on others in the meantime. @CCiufo-WMF and DST can decide what naming convention makes the most semantic sense for dark mode image backgrounds. If we decide we don't want to wait, we can use the hex described in the task description, mark it as tech debt, and update it to the design token when it's available.

I would recommend not tokenizing this value for now. This seems like a very specific use case that should live as a decision in the skin until we either find more uses for it or it becomes more "stable". I have a feeling the color might change as we get more feedback about how it behaves with different SVGs and I don't think a dependency on Codex there makes sense. Does that work for you all? We can still add color-base-fixed as requested in T371529.

In T370074#10035630, @CCiufo-WMF wrote:

In T370074#10033801, @JScherer-WMF wrote:

In T370074#10033615, @Jdlrobson wrote:

Hey @JScherer-WMF please see T371529#10033077 - DST are concerned that we are using an inappropriate token for the background here.

I'm fine to wait for a new token for this and mark this as blocked on others in the meantime. @CCiufo-WMF and DST can decide what naming convention makes the most semantic sense for dark mode image backgrounds. If we decide we don't want to wait, we can use the hex described in the task description, mark it as tech debt, and update it to the design token when it's available.

I would recommend not tokenizing this value for now. This seems like a very specific use case that should live as a decision in the skin until we either find more uses for it or it becomes more "stable". I have a feeling the color might change as we get more feedback about how it behaves with different SVGs and I don't think a dependency on Codex there makes sense. Does that work for you all? We can still add color-base-fixed as requested in T371529.

That makes sense from my perspective. I agree this feels too specific to be a token, and I don't see where else we'd use it. Thanks!

Change #1058619 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Dark mode: Images should have background

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

Change #1059438 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Dark mode: Images should have background

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

@bwang since I'm out next week, I'm passing this to you.

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1058643 should be ready to merge. Once you've merged that, please move to QA and assign to Edward for some exploratory testing (desktop only)

I suggest that we merge https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/1059438 on Thursday next week together - e.g. ship to desktop only next week, in case we want to refine the selectors based on desktop. Sound good?

Change #1058643 merged by jenkins-bot:

[mediawiki/core@master] File pages: Apply background in dark mode to file pages

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

@Jdlrobson @ovasileva Please review errors. These images are for PROD based on the links provided in the QA test steps. Part 1 of 2. Images are still dark.

Website	Desktop	Desktop w/?useparsoid=1	Mobile	Mobile w/?useparsoid=1
https://en.wikipedia.org/wiki/L-DOPA	❌	❌	❌	❌
https://en.wikipedia.org/wiki/The_arts#Music	✅	✅	✅	✅
https://en.wikipedia.org/wiki/Human#Gender	✅	❌	✅	✅
https://en.wikipedia.org/wiki/Forceps#Thumb_forceps	✅	✅	✅	✅
https://en.wikipedia.org/wiki/Greek_alphabet#Diacritics	✅	✅	✅	✅
https://en.wikipedia.org/wiki/Chroma_subsampling	✅	❌	✅	✅
https://en.wikipedia.org/wiki/Hexadecimal#Verbal_and_digital_representations	✅	✅	✅	✅

@Jdlrobson @ovasileva These images are for PROD based on the links provided in the QA test steps. Part 2 of 2 Errors missing background color #C8CCD1

Website	Desktop	Desktop w/?useparsoid=1	Mobile	Mobile w/?useparsoid=1
https://en.wikipedia.org/wiki/Homotopy_lifting_property#Formal_definition	✅	✅	✅	✅
https://en.wikipedia.org/wiki/Phase-change_memory#PRAM_vs._Flash	✅	✅	✅	✅
https://en.wikipedia.org/wiki/Oblique_projection?debug=1#Oblique_pictorial	✅	✅	✅	✅
https://en.wikipedia.org/wiki/UConn_Huskies_women%27s_basketball	✅	❌	✅	✅
https://en.wikipedia.org/wiki/Indiana_Hoosiers_men%27s_basketball	✅	❌	✅	✅
https://en.wikipedia.org/wiki/Ketosis	✅	❌	✅	✅
https://en.wikipedia.org/wiki/Earth	✅	❌	✅	✅
https://en.wikipedia.org/wiki/File:Indian_General_Election_2024.svg	❌	❌	❌	❌

Change #1059438 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Dark mode: Images should have background

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

Hi @Edtadros I'm not sure how we can QA this in the beta cluster, but we can QA it in production.

The fix is in production for desktop as of today, and will be in mobile from next Thursday.

With that in mind would it make sense to QA this as part of T371113 ?

@Jdlrobson Tested in Desktop only per T370074#10050791 in PROD. There are no issues with background color as shown in the screenshots. I will move this to sign-off, thanks!

Test Result - PROD

Status: ✅ PASS
Environment: PROD
OS: macOS Sonoma 14.6
Browser: Chrome 127
Device: MBA
Emulated Device: NA

Website	Desktop w/?useparsoid=0	Desktop w/?useparsoid=1
https://en.wikipedia.org/wiki/L-DOPA
https://en.wikipedia.org/wiki/The_arts#Music
https://en.wikipedia.org/wiki/Human#Gender
https://en.wikipedia.org/wiki/Forceps#Thumb_forceps
https://en.wikipedia.org/wiki/Greek_alphabet#Diacritics
https://en.wikipedia.org/wiki/Chroma_subsampling
https://en.wikipedia.org/wiki/Hexadecimal#Verbal_and_digital_representations
https://en.wikipedia.org/wiki/Homotopy_lifting_property#Formal_definition
https://en.wikipedia.org/wiki/Phase-change_memory#PRAM_vs._Flash
https://en.wikipedia.org/wiki/Oblique_projection?debug=1#Oblique_pictorial
https://en.wikipedia.org/wiki/UConn_Huskies_women%27s_basketball
https://en.wikipedia.org/wiki/Indiana_Hoosiers_men%27s_basketball
https://en.wikipedia.org/wiki/Ketosis
https://en.wikipedia.org/wiki/Earth
https://en.wikipedia.org/wiki/File:Indian_General_Election_2024.svg

Thanks for applying the background. What about images in galleries? The gallery in https://de.wikipedia.org/wiki/Butylgruppe?useskin=vector-2022&vectornightmode=1 could also benefit from this.

Is this resolved in production? We are still having readers complain about this problem:

https://ticket.wikimedia.org/otrs/index.pl?Action=AgentTicketZoom;TicketID=13317243

It has but it intentionally doesn't apply to infoboxes. For issues like this, it's best to let the talk page know.
https://en.wikipedia.org/wiki/Template:Infobox_drug

When posting to the talk page, let them know that infoboxes support an imagestyle and imageclass property now (https://en.wikipedia.org/w/index.php?title=Template%3AInfobox_drug&diff=1244184629&oldid=1189170641) which can be easily used to fix this (I've fixed this particular template!)

Thanks for your help with OTRS @Xaosflux - feel free to ping me on my volunteer account (User:Jdlrobson) and I will happily help fix the most common ones to reduce the tickets you are getting in my off time!

I did not really understand why the case of Infobox was separated from the others. It is true that we can fix it using skin-invert class, but the same applies outside Infobox.

Other question: would it be possible to extend the grey background to previews from the Popups extension? In the case of transparent backgrounds, skin-invert class is not used even when it is applied inside the previewed article.
Same issue with the search (from the search box as well as in Special:Search).

In T370074#10140687, @Escargot_rouge wrote:

I did not really understand why the case of Infobox was separated from the others. It is true that we can fix it using skin-invert class, but the same applies outside Infobox.

It boils down to:

1. Some transparent SVGs are best inverted
2. Some transparent SVGs should be left alone
3. Some transparent SVGs need a background (either white or some other color)

Given the nuance here, it seemed better to address this as the template level as the majority of cases fall into category 1 or 2 and 2 requires no changes from an editor. In short: applying a background to infoboxes creates more work for all our communities. Hope this makes sense?

Other question: would it be possible to extend the grey background to previews from the Popups extension? In the case of transparent backgrounds, skin-invert class is not used even when it is applied inside the previewed article.
Same issue with the search (from the search box as well as in Special:Search).

We haven't actually considered the page previews case so that definitely seems worthy of a task and that seems like an example where white backgrounds could make sense (or some other solution) since editors cannot influence it right now.