MediaWiki r105280 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r105279‎ | r105280 (on ViewVC)‎ | r105281 >
Date:09:41, 6 December 2011
Author:hashar
Status:reverted (Comments)
Tags:design 
Comment:
Diff colors now use the french Wikipedia scheme

The french community has been using a specific set of colors for diff, it is
believed to be easier to read for people perceiving colors differently.

Source is from:
http://fr.wikipedia.org/w/index.php?oldid=72567845&uselang=en

This commit override r94429 / r94461.

See also docs/uidesign/mediawiki.action.history.diff.html
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r106884Followup r105280...bharris22:53, 20 December 2011
r107127Applying patch from bug 33335 (from Erwin Dokter). Followup to r105280...robla00:22, 23 December 2011
r108975[mediawiki.action.history.diff.css] remove redundant css rule...krinkle13:56, 15 January 2012
r112750[mediawiki.action.history.diff.css] Revert 1.19 style changes back to how it ...krinkle01:07, 1 March 2012
r112836Resolves bug #11374 and improves on r94429, r94461, r101147, r105280, r106884...tparscal21:27, 1 March 2012

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r94429(bug 11374) Red .diffchange text in the green 'added' area may be hard to rea...maxsem20:57, 13 August 2011
r94461Fix r94429 : Left and right side are still nearly the same color for 7% of th...diebuche14:40, 14 August 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   18:26, 6 December 2011

I'm pretty happy with this -- marking ok.

We should probably make sure there's a gadget to reenable the "classic" diff colors for grumpy people who dislike change. :)

#Comment by Hashar (talk | contribs)   21:28, 6 December 2011

A gadget would be great. Never code one myself though so I leave this task to someone else. Thanks for the fast review!

#Comment by Edokter (talk | contribs)   01:33, 7 December 2011
#Comment by Brion VIBBER (talk | contribs)   01:46, 7 December 2011

awesome :D

#Comment by Hashar (talk | contribs)   06:48, 7 December 2011

I have no idea how you came here or heard about this revision, but anyway thanks a lot for that Edokter!

  • sends wikilove *
#Comment by Edokter (talk | contribs)   11:11, 7 December 2011

You're welcome :) I heard because Brion spilled the beans on wikitech-l.

#Comment by Jarry1250 (talk | contribs)   16:15, 12 December 2011

I assume we're not going regret making green change its meaning from added to removed?

#Comment by Hurricanefan25 (talk | contribs)   13:16, 14 December 2011

Can someone switch green to "removed" and switch around the other stuff? It's confusing :S

#Comment by Tagishsimon (talk | contribs)   13:23, 14 December 2011

Agreed. It's one thing to change colours to improve accessibility. Another entirely, for no good reason, to move green to "removed" merely because that's how things stand on the French wikipedia. Better surely to leave green where it was and merely change the yellow to blue? Moving green to "remove" is a wholly unnecessary change.

#Comment by Edokter (talk | contribs)   14:01, 14 December 2011

Doing so would shift that ambiguity to the color-blind, most of whom cannot distinguish between red and green. So it would make even less sense to them to swap the colors. I don't even rely on the colors; to me, left means removed, right means added.

#Comment by Taylornate (talk | contribs)   14:34, 14 December 2011

What? No it wouldn't. Either they can distinguish the green and blue or they can't.

#Comment by Edokter (talk | contribs)   16:39, 14 December 2011

That's not the point; red and green are perceived as a shade of orange, so orange for removal/blue for addition makes more sense.

#Comment by Catrope (talk | contribs)   16:44, 14 December 2011

I agree that we should cater to the colorblind by picking colors they can distinguish, but when deciding which color means what, surely we should prioritize what makes sense to the general population over what makes sense to colorblind people? I'm all for colorblind accessibility, but when deciding whether or not to swap the colors surely we shouldn't pick a scheme that's counterintuitive for the 95%+ that aren't colorblind just so it makes marginally more sense to those that are?

#Comment by Taylornate (talk | contribs)   16:48, 14 December 2011

That makes absolutely no sense because if someone cannot distinguish red/green then they are not going to arbitrarily associate the resulting color to mean "remove". In fact, if they can differentiate at all between green/yellow, then they will be used to green meaning "add" just like the rest of the population.

#Comment by Catrope (talk | contribs)   16:50, 14 December 2011

I can see Edokter's point that if green and red both look like orange, you might associate that with removal rather than addition, so in that regard it somewhat makes sense. But what doesn't make sense is for the majority to get the colors backwards just so it looks marginally more sensible to a small minority.

#Comment by Taylornate (talk | contribs)   16:55, 14 December 2011

No, red/green/orange will not magically mean red to someone who cannot tell the difference. "Orange" is not going to mean the same thing to a colorblind person as it will to the rest of us. It will just be that color that sometimes means stop and other times means go.

#Comment by Catrope (talk | contribs)   16:57, 14 December 2011

Sure. But what I'm saying is that regardless of whether red/green/orange makes sense to colorblind people, we shouldn't assign the colors in such a way that they're perceived as backwards by the non-colorblind majority.

#Comment by Taylornate (talk | contribs)   17:01, 14 December 2011

I agree. I guess I was responding to your thought that his point made sense.

#Comment by Catrope (talk | contribs)   17:03, 14 December 2011

Yeah, you're probably right about that one, it makes less sense than I thought it did.

#Comment by Darkoneko (talk | contribs)   11:29, 15 December 2011

The non-colourblind majority of frwiki has had that colour scheme since 2006, and I have yet to hear any complain about green being left from them.

Hance, I feel there is a group bias here, tho I'm not sure why. It might be because on special:code, diffs are show with red on left/green on right and most people here are used to it ?

#Comment by Jarry1250 (talk | contribs)   11:39, 15 December 2011

Oh, no. It's just that the French changed *in 2006*. They weren't used to the diff colours nearly as much. en.wp and other wikis (WMF and non-WMF) now are, which begs the question (in the non-technical sense), why change just for the sake of change. If we can improve the situation for colourblind users without swapping the meaning of green for them, why shouldn't we?

#Comment by Darkoneko (talk | contribs)   23:15, 15 December 2011

I have to disagree with your 2006 argumentation : The average life expectancy of an active wikimedia editor is (if I remember that WMF study correctly) around 18 months ; be it in 2006 or 2011, half the editors have been there for that time or less ; Even now I expect a most of the rest to be under 3 to 4 years old.

Please note that I don't have a real preference for either way. Like hashar said below, it was pretty random... I didn't want red, which left the 2 other primary colours, green and blue. The real point of that code is to highlight the changed parts better, aka setting a different background colour rather than just putting the changed text in red. Having the whole paragraph in a much lighter version of that background, or having a different background colour on left and on right ? Those only serves as eye candies.


Your later question is the same as mine, as I still don't agree this "green must be on the right" reasoning.

I take the issue from another perspective : frwiki isn't the only one using the green/blue scheme; several other wikis have since reused that code into their own CSS From a quick (and incomplete) search on the wikipedia.org domains, at least af:, ar:, bs:, da:, he:, lv:, nl:, oc:, pt:, sl: and sr: uses it too.

Currently, we have : -yellow #FFFFAA (changed text in red) / lightgreen #CCFFCC (changed text in red) the current default -lightgreen #E4F6D8 / lightblue #D8E4F6 (on various wikis), the proposed version

those are two pretty distinct schemes, people shouldn't have problem differentiate them

If we go with the inverted version, we will have : -lightblue #D8E4F6 / lightgreen #E4F6D8 (new default) -lightgreen #E4F6D8 / lightblue #D8E4F6 (on various wikis)

these 2 schemes are just the opposite of one another, I expect them to confuse the hell out of any multi-wiki editor

#Comment by Darkoneko (talk | contribs)   23:22, 15 December 2011

(uh, sorry, it's just unreadable without the correct line separation. Someone please delete the misformated duplicate)

I have to disagree with your 2006 argumentation : The average life expectancy of an active wikimedia editor is (if I remember that WMF study correctly) around 18 months ; be it in 2006 or 2011, half the editors have been there for that time or less ; Even now I expect a most of the rest to be under 3 to 4 years old.

---

Please note that I don't have a real preference for either way. Like hashar said below, it was pretty random... I didn't want red, which left the 2 other primary colours, green and blue.

The real point of that code is to highlight the changed parts better, aka setting a different background colour rather than just putting the changed text in red. Having the whole paragraph in a much lighter version of that background, or having a different background colour on left and on right ? Those only serves as eye candies.

---

Your later question is the same as mine, as I still don't agree this "green must be on the right" reasoning.

Taking the issue from another perspective : frwiki isn't the only one using the green/blue scheme; several other wikis have since reused that code into their own CSS. From a quick (and incomplete) search on the wikipedia.org domains, at least af:, ar:, bs:, da:, he:, lv:, nl:, oc:, pt:, sl: and sr: uses it too.

Currently, we have :

  • yellow (changed text in red) / lightgreen (changed text in red), the current default
  • lightgreen #E4F6D8 (changed text with green background) / lightblue (changed text with blue background), currently on various wikis, and the proposed version

those are two pretty distinct schemes, people shouldn't have problem differentiate them.

If we go with the inverted version, we will have :

  • lightblue / lightgreen (new default)
  • lightgreen / lightblue (on various wikis)

these 2 schemes are just the opposite of one another, I expect them to confuse the hell out of any multi-wiki editor.

#Comment by Edokter (talk | contribs)   23:29, 15 December 2011

I think most wikis will remove their CSS green/blue once the new default is available.

#Comment by Darkoneko (talk | contribs)   23:46, 15 December 2011

I don't think they will.

For them, it would mean inverting their usual colour scheme just for the sake of it, without a real purpose, since they already solved the readability issue

#Comment by David Levy (talk | contribs)   01:36, 16 December 2011

For a vast majority of wikis, switching the green background's location would invert its meaning just for the sake of it, without a real purpose. That a readability issue would be solved simultaneously is irrelevant, as that can occur without inverting the meaning of green.

#Comment by Darkoneko (talk | contribs)   01:58, 16 December 2011

but I feel that most people (me included) don't give any particular meaning of which colour is on which side. It's probably why I have a hard time understanding that issue

#Comment by David Levy (talk | contribs)   02:10, 16 December 2011

Based on the responses received here and at the English Wikipedia, your feeling appears to be incorrect.

#Comment by Darkoneko (talk | contribs)   10:37, 16 December 2011

Hmm, could you link to where this is discussed on the english Wikipedia ? This could be used for reference too

#Comment by David Levy (talk | contribs)   01:33, 16 December 2011

It's true that many users come and go, but that has no bearing on the ones that stay. The longer a wiki exists, the longer its most experienced editors have been there. At a vast majority of Wikimedia wikis, those users are accustomed to the green background carrying a particular connotation in diffs.

You appear to recognize the distinction between adjusting to something entirely new and adjusting to something old taking on a new meaning. Because the blue background is new to most of us, its inclusion isn't terribly jarring. Conversely, inverting the green background's meaning is quite jarring.

I assume, as you state, that the same applies to editors of the French Wikipedia and the handful of other Wikimedia wikis that have adopted its diff code (most likely without regard for the coloration, as briefly occurred at the English Wikipedia in 2008). But it simply isn't reasonable to request that the vast majority of wikis bend to conform with a small minority.

Keep in mind, however, that we're merely discussing the default. Irrespective of what decision is made, any wiki is free to retain/adopt custom code. If the default meaning of green is inverted, it's highly likely that the English Wikipedia and other wikis will switch it back locally. So either way, we won't have diff uniformity across 100% of wikis. (Of course, that's been the case for years.) But given the relatively small number of wikis using the French color scheme (some of which probably will keep their custom code), we stand to come substantially closer to attaining uniformity by retaining the longstanding default meaning of green (which most of the wikis have used for years).

#Comment by Taylornate (talk | contribs)   01:36, 16 December 2011

To further support what you are saying, green=removed is going to seem backwards to new users who have never used a wiki before.

#Comment by Darkoneko (talk | contribs)   01:51, 16 December 2011

I ... don't really see why ?

#Comment by David Levy (talk | contribs)   02:11, 16 December 2011

Green traditionally indicates something good, such as forward progress (as with a traffic light).

#Comment by Darkoneko (talk | contribs)   10:35, 16 December 2011

Blue is also a typical "good" indicator in medias [1] - think Tron, Star Wars, etc. (and countries like Germany uses blue as "forward" in traffic lights)

#Comment by David Levy (talk | contribs)   12:25, 16 December 2011

Blue doesn't traditionally carry a negative connotation (and it sometimes takes on a positive connotation when contrasted with other colors — notably red), but compared to green, "good" symbolism isn't nearly as common (particularly when color-coding text, icons and emblems).

In Star Wars, both blue and green often indicate good (with red indicative of evil).

Tron's original script called for blue to indicate evil. In the actual film, the usage is mixed (good and evil).

I was unable to find any information about the traffic lights to which you refer.

#Comment by Edokter (talk | contribs)   19:23, 14 December 2011

This change is now gaining a lot of opposition on Wikipedia, especially concerning the 'green=remove' scheme. How about we swap the colors and restore the gray background for context lines? (I personally don't like the white.)

#Comment by Jarry1250 (talk | contribs)   19:26, 14 December 2011

Sounds good. Alternatively, it could just be swapped for existing/Wikimedia wikis.

#Comment by Brion VIBBER (talk | contribs)   19:26, 14 December 2011

I've honestly never paid much attention to which color is which, especially on this style where blue and green neither have any particular opposition meaning to each other. No objection to changing, or to not changing, on my part.

The side (left = before, right = after) and the - and + characters, are the primary indicators of what's removed vs added.

#Comment by David Levy (talk | contribs)   20:35, 14 December 2011

I strongly agree that if these colors are to be used, they should be swapped (so that the left column is blue and the right one remains green). This would be significantly more intuitive without impacting accessibility. As I noted when the English Wikipedia briefly adopted the French diff style in 2008, there's no need to suddenly reverse the meaning of green.

#Comment by Danhash (talk | contribs)   21:18, 14 December 2011

I'm glad that someone is thinking about accessibility for color blind users, but this is an utterly terrible change. The colors should either be swapped or other colors used.

#Comment by David Levy (talk | contribs)   21:37, 14 December 2011

When the English Wikipedia briefly adopted the French diff style in 2008 (before it was determined that the change lacked consensus), I adjusted it to incorporate the familiar color scheme (amber on the left and green on the right). Here's a screen capture.
I've used that code in my personal CSS ever since. Does it present any accessibility issues? If not, it seems like a better solution.

#Comment by Darkoneko (talk | contribs)   11:27, 15 December 2011

Unfortunatly, the code indicate that the left and the right side are of different colours, but I fail to see any difference ^^; (I suffer from protanomaly)


On an unrelated note : on your screenshot, the whole lines are changed text, if would be a better showcase to have both changed text (with distinct background) and normal text (with normal background) on the same line

#Comment by David Levy (talk | contribs)   01:42, 16 December 2011

Here's a better example.

To be clear, my priority is that we not suddenly invert the default meaning of green. Replacing amber with blue is a relatively minor change (and if it assists colorblind people, that obviously is a good thing).

#Comment by Edokter (talk | contribs)   21:37, 14 December 2011

Also, any objections to restoring the gray background for context (empty) lines to gray? I'll prepare a patch.

#Comment by Jarry1250 (talk | contribs)   21:48, 14 December 2011

Black on #EEE passes WCAG (comfortably), so I can't see why not, at least for WMF wikis.

#Comment by Edokter (talk | contribs)   23:38, 14 December 2011

Patch to swap colors at Bug 33139.

#Comment by Peachey88 (talk | contribs)   04:59, 15 December 2011

As I noted on the mailing list, This diff mixes two de-facto "standards" for colour coding "warnings", The standard one that most of us are used to the and the more colour-blind friendly version.


We need to pick on version and stick with it, instead of mix and matching two sets (and then also changing the meaning of a colour)


Table of colours:

Common/"De-facto Standard" Colour-Blind standard(ish) This change
Good Green Yellow Blue
Bad Red Blue Green
#Comment by Darkoneko (talk | contribs)   21:57, 15 December 2011

Colour-blind standart is the (old diff) yellow background ? I barely can distingish black text from red text on that

#Comment by Peachey88 (talk | contribs)   23:12, 15 December 2011

Those standards aren't diff specific, they are just the everyday ones, so text on them might not considered all that well. I'm just aware of the Yellow/Blue because I was taught it because some of the special needs resourced I used had used that instead of the Green/Red schema.

#Comment by Darkoneko (talk | contribs)   01:27, 16 December 2011

Ah, I see what you mean now. But now that I've written a bit about it above [1], I realise we have been sidetracked.

We have been concentrating on the left/right size colour difference, but the readability issue has nothing to do with these. It's the same colour for the whole paragraph ! It doesn't help seeing what changed, so it could be any colour as long as there enough contrast with the text.


What the change is really about is the method we differentiate the unchanged and altered text parts inside that paragraph

  • the current scheme uses a different 『text colour』 (red instead of black)
  • the proposed scheme instead uses a different 『text background colour』for the altered part


So, the important part is how the "altered text" background is different from the "normal text" background ; how the left side's is different from the right side's is unrelated.

---

To develop a bit on the why and how of the two differentiation methods :

The current method relies on a red/black text change ; this makes it impossible or very hard to see the difference for people with a "red" related deficiency.

The proposed method relies on :

  • a more obvious contrast zone (by doing affecting the whole line rather of just the text)
  • this contrast is made on all three primary colours. It's easy to see as CSS colours code are in RGB format
    • on the (current) left side, #E4F6D8 (paragraph) is contrasted with a darker #B0E897 (altered text)
    • on the (current) right side, #D8E4F6 (paragraph) is contrasted with a darker #B0C0F0; (altered text)

To be honest, the best contrast would be against a white background paragraph ; having it lightly coloured is an eye-candy compromise. But even so, it should be differentiable by people suffering from all type of colour blindness, including monochromia.

#Comment by David Levy (talk | contribs)   01:59, 16 December 2011

We've concentrated on the background colors because the inversion of green's default meaning is the only change to which many people have objected (this time, at least).

I can't speak for others, but I strongly prefer the French Wikipedia's differentiation method. As noted above, I've used it via my personal CSS (albeit with a different color scheme) since 2008.

That the controversy pertains to a peripheral element (not the actual improvement) is a good thing.  :)

#Comment by Hashar (talk | contribs)   11:37, 15 December 2011

I have talked with Darkoneko from the french wiki (he added comments above): the color have apparently been chosen randomly. He has protanomaly and told me there is not justification to have green on left and blue on right. See his comment https://www.mediawiki.org/wiki/Special:Code/MediaWiki/105280#c27748

French Wikipedia had the green color on left for the last 5 or 6 years and that did not raise any issue. As I interprets it, we could consider that the left part is the validated text and the right part is the change pending validation. So that the green on left (validated) and blue on right (pending) would make sense :-D

I personally don't care. Would be glad to apply Edokter patch on bug 33139 if the community reach a consensus.

#Comment by Taylornate (talk | contribs)   23:59, 15 December 2011

If French users did not have an issue with it (assuming that's true), that's great but irrelevant. The fact is, there are people here who have an issue with it. I understand that French/English editors may have to adjust, but they are a minority and it makes more sense to have them adjust or patch rather than everyone else, including new users.

I also like the yellow/blue combo someone mentioned.

#Comment by Edokter (talk | contribs)   12:24, 15 December 2011

Really, personally I don't care which color is left/right. My patch just swaps them (and resoted the gray context background).

While we're at at, let's finish the job properly and get it ready for 1.19. I made some further tweaks that makes white-space:pre-wrap apply to the entire diff cell instead of only the .diffchange part. This will show off indents quite nicely. I have a patch ready (so hold on before applying bug 33139).

Second, bug 25697 has been open for a year and a half, waiting for a patch review that will fix the empty context row issue (that collapses an entire row if no cell has content). If we can please have that fixed, then I would consider diffs to be truly "done".

#Comment by Edokter (talk | contribs)   12:30, 15 December 2011

To test the pre-wrap (and colors) in action, see the CSS marked /* Diffs */ in my vector.css at.

#Comment by Edokter (talk | contribs)   12:31, 15 December 2011

Fixed link: en:User:Edokter/vector.css (I really miss preview/edit here.)

#Comment by David Levy (talk | contribs)   04:56, 16 December 2011

Via IM, I just showed a friend (an occasional Wikimedia editor and administrator at an independent MediaWiki-based wiki) an example diff from the French Wikipedia.

Friend: I see. Not a fan.
Me: Any particular issues?
Friend: Green is opposite in meaning, for one thing. The colors aren't intuitive.

#Comment by Hashar (talk | contribs)   07:14, 16 December 2011

Tagging 'fixme' so we remember to not deploy that right now and need to reach a consensus. ;)

#Comment by Edokter (talk | contribs)   10:41, 16 December 2011

Consensus seems clear to me: *the* biggest issue was the 'reversed' meaning of green. That's why I swapped the colors. I have the new scheme installed as a gadget on enwiki, and response has been positive. I'm going to go ahead ans submit the patch. And *please* will someone look at bug 25697 too? It's been stale for no reason.

#Comment by Edokter (talk | contribs)   01:17, 21 December 2011

Discussion continues at r106884.

#Comment by Hashar (talk | contribs)   10:07, 21 December 2011

Marking this revision as resolved. The main issue was the green color on the left side. Follow up r106884 fix that by removing green entirely and replace it with yellow.

#Comment by Fomafix (talk | contribs)   17:24, 8 January 2012

table, td { background-color: transparent; } is not necessary because this is the initial value.

#Comment by Fomafix (talk | contribs)   08:42, 11 January 2012

Since r80495 tables have no more white background color.

Status & tagging log

  • 01:07, 1 March 2012 Krinkle (talk | contribs) changed the status of r105280 [removed: resolved added: reverted]
  • 09:53, 2 January 2012 Krinkle (talk | contribs) changed the tags for r105280 [removed: frontend]
  • 10:13, 21 December 2011 Aaron Schulz (talk | contribs) changed the status of r105280 [removed: new added: resolved]
  • 10:08, 21 December 2011 Hashar (talk | contribs) changed the status of r105280 [removed: fixme added: new]
  • 07:14, 16 December 2011 Hashar (talk | contribs) changed the status of r105280 [removed: ok added: fixme]
  • 07:13, 16 December 2011 Hashar (talk | contribs) changed the tags for r105280 [added: frontend]
  • 18:26, 6 December 2011 Brion VIBBER (talk | contribs) changed the status of r105280 [removed: new added: ok]
  • 09:43, 6 December 2011 Hashar (talk | contribs) changed the tags for r105280 [added: design]