r112750 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112749‎ | r112750 | r112751 >
Date:01:07, 1 March 2012
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
[mediawiki.action.history.diff.css] Revert 1.19 style changes back to how it was in REL1_18
* After the 1.19wmf1 deployment several people have complained about various aspects of the new diff styling
-- The contrast being too low in the highlighted part (darker background behind bolded text)
-- The colors not being obvious perhaps (orange/blue)
-- Color blind users not seeing the difference very well between the light tones of the orange and blue
-- Trevor mentioned something about W3C Accessibility guidelines

To play it safe for now I think we should revert these changes to the status quo, and take the next few days (or weeks) to carefully check the concerns, perhaps look at other diff tools out there for inspiration (GitHub, Gerrit, LocalWiki, ..).

* Goes back to r91762 (last rev before REL1_18)
-- reverting: r94429, r94461, r101147, r105280, r106884, r107127, r108975, r109932
* Re-opens bug 33335
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/resources/mediawiki.action/mediawiki.action.history.diff.css (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -92,8 +92,6 @@
9393 "target" query parameter (eg. Special:ActiveUsers/Username)
9494 * New JavaScript variable wgPageContentLanguage
9595 * Added new debugging toolbar, enabled with $wgDebugToolbar
96 -* (bug 11374) Differences in the history page now uses slightly better colors for people
97 - perceiving colors differently.
9896 * (bug 32879) Upgrade jQuery to 1.7.1
9997 * jQuery UI upgraded to 1.8.17
10098 * Extensions can use the 'Language::getMessagesFileName' hook to define new
Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.history.diff.css
——@@ -1,54 +1,39 @@
2 -/**
3 - * Diff rendering
4 - */
5 -
 2+/*
 3+** Diff rendering
 4+*/
 5+table.diff, td.diff-otitle, td.diff-ntitle {
 6+ background-color: white;
 7+}
68 td.diff-otitle,
79 td.diff-ntitle {
810 text-align: center;
911 }
10 -
1112 td.diff-marker {
1213 text-align: right;
1314 }
14 -
1515 td.diff-lineno {
1616 font-weight: bold;
1717 }
18 -
19 -td.diff-addedline,
20 -td.diff-deletedline,
21 -td.diff-context {
22 - font-size: 88%;
23 - vertical-align: top;
24 - white-space: -moz-pre-wrap;
25 - white-space: pre-wrap;
26 -}
27 -
2818 td.diff-addedline {
29 - background: #E0ECFF;
 19+ background: #cfc;
 20+ font-size: smaller;
3021 }
31 -
3222 td.diff-deletedline {
33 - background: #FCF8CC;
 23+ background: #ffa;
 24+ font-size: smaller;
3425 }
35 -
3626 td.diff-context {
37 - background: #F2F2F2;
 27+ background: #eee;
 28+ font-size: smaller;
3829 }
39 -
4030 .diffchange {
 31+ color: red;
4132 font-weight: bold;
 33+ white-space: -moz-pre-wrap;
 34+ white-space: pre-wrap;
4235 text-decoration: none;
4336 }
4437
45 -td.diff-addedline .diffchange {
46 - background: #B0C8FF;
47 -}
48 -
49 -td.diff-deletedline .diffchange {
50 - background: #FFD084;
51 -}
52 -
5338 table.diff {
5439 border: none;
5540 width: 98%;
——@@ -57,25 +42,20 @@
5843 /* Ensure that colums are of equal width */
5944 table-layout: fixed;
6045 }
61 -
6246 table.diff td {
6347 padding: 0;
6448 }
65 -
6649 table.diff col.diff-marker {
6750 width: 2%;
6851 }
69 -
7052 table.diff col.diff-content {
7153 width: 48%;
7254 }
73 -
7455 table.diff td div {
7556 /* Force-wrap very long lines such as URLs or page-widening char strings.*/
7657 word-wrap: break-word;
7758
7859 /* As fallback (FF<3.5, Opera <10.5), scrollbars will be added for very wide cells
79 - * instead of text overflowing or widening
80 - */
 60+ instead of text overflowing or widening */
8161 overflow: auto;
8262 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r112763MFT r112750reedy02:01, 1 March 2012
r112836Resolves bug #11374 and improves on r94429, r94461, r101147, r105280, r106884...tparscal21:27, 1 March 2012
r113040MFT r111427, r112347, r112374, r112383, r112700, r112750, r112855reedy15:15, 5 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r91762Undo browser defaults (underline/strike) for the <del> and <ins> DairikiDiff ...krinkle21:32, 8 July 2011
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
r101147* use outline instead of border to not change the dimensions of the elementdanny_b16:15, 28 October 2011
r105280Diff colors now use the french Wikipedia scheme...hashar09:41, 6 December 2011
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
r109932diff: align cell content to the top...hashar16:25, 24 January 2012

Comments

#Comment by Trevor Parscal (WMF) (talk | contribs)   01:56, 1 March 2012

A new patch that follows W3C Accessiblity guidelines will follow (authored by Timo and I)

The reasons these changes need to be reverted are because of violations of guidelines.

1. Don't rely on color along 2. Contrast ratio between background and foreground colors on the changed text portion of a diff is not high enough

See: http://www.w3.org/TR/WCAG10/

#Comment by MZMcBride (talk | contribs)   02:46, 1 March 2012

Can you copy this to Bugzilla (specifically bug 11374)? (And attach any patches, etc. there?) The fractured discussion across revisions is becoming difficult to follow.

#Comment by Rillke (talk | contribs)   12:02, 1 March 2012

This revert is was completely unnecessary. Now we have red text on green background again with 0 contrast. Dots, small chars and white spaces are not shown. Very bad!

The new diffs were something I liked most regarding to MW 1.19.

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:09, 1 March 2012

A new patch is coming which will provide the same functionality as these changes did, but without breaking accessibility.

The extra features you mention (rendering spaces changed and making dots easier to notice) are excellent reasons to make a change. Unfortunately this set of changes fixed those at the cost of other issues, making the user interface less accessible to visually impaired people.

Status & tagging log