r22204 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r22203‎ | r22204 | r22205 >
Date:17:57, 16 May 2007
Author:brion
Status:old
Tags:
Comment:
* (bug 1438) Fix for diff table layout on very wide lines.
Diff style rules have been broken out to common/diff.css,
and the dupes removed from the default skin files.
Skins can still override the default rules.

Improvements over r22192, now known to work in:
* Firefox 2.0.0.3
* Opera 9.10 and 9.20
* Safari 2.0.4
* Konqueror 3.5.6
* MSIE/Win 7.0 (wide cells may produce vertical scrollbars as well)
* MSIE/Win 6.0 (wide cells are cropped instead of scrolling)
* MSIE/Mac 5.2.3 (wide words break instead of scrolling)
* iCab 3.0.3 (some cells provoke unnecessary horizontal scrollbar)

I've cleaned up the diff table formatting a bit, moving some attributes
from the HTML to the style sheet and consolidating the duplicated styles
into a common/diff.css file which is conditionally loaded for diff views.

Individual skins or site/user CSS can still override that style if they wish.
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/skins/mw-skins/ontoskin/handheld.css (modified) (history)
  • /trunk/extensions/sampleskin/main.css (modified) (history)
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/skins/chick/main.css (modified) (history)
  • /trunk/phase3/skins/common/common.css (modified) (history)
  • /trunk/phase3/skins/common/diff.css (added) (history)
  • /trunk/phase3/skins/monobook/handheld.css (modified) (history)
  • /trunk/phase3/skins/monobook/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/chick/main.css
——@@ -397,25 +397,6 @@
398398 background-color:#f9f9f9;
399399 border:1px dashed #aaaaaa;
400400 }
401 -/*
402 -** Diff rendering
403 -*/
404 -table.diff { background:white; }
405 -td.diff-otitle { background:#ffffff; }
406 -td.diff-ntitle { background:#ffffff; }
407 -td.diff-addedline {
408 - background:#ccffcc;
409 - font-size: smaller;
410 -}
411 -td.diff-deletedline {
412 - background:#ffffaa;
413 - font-size: smaller;
414 -}
415 -td.diff-context {
416 - background:#eeeeee;
417 - font-size: smaller;
418 -}
419 -.diffchange { color: red; text-decoration: none; }
420401
421402 a.external { color: #3366bb; }
422403
Index: trunk/phase3/skins/monobook/handheld.css
——@@ -542,29 +542,6 @@
543543 }
544544
545545 /*
546 -** Diff rendering
547 -*/
548 -table.diff, td.diff-otitle, td.diff-ntitle {
549 - background-color: white;
550 -}
551 -td.diff-addedline {
552 - background: #cfc;
553 - font-size: smaller;
554 -}
555 -td.diff-deletedline {
556 - background: #ffa;
557 - font-size: smaller;
558 -}
559 -td.diff-context {
560 - background: #eee;
561 - font-size: smaller;
562 -}
563 -.diffchange {
564 - color: red;
565 - font-weight: bold;
566 -}
567 -
568 -/*
569546 ** keep the whitespace in front of the ^=, hides rule from konqueror
570547 ** this is css3, the validator doesn't like it when validating as css2
571548 */
Index: trunk/phase3/skins/monobook/main.css
——@@ -582,34 +582,6 @@
583583 }
584584
585585 /*
586 -** Diff rendering
587 -*/
588 -table.diff, td.diff-otitle, td.diff-ntitle {
589 - background-color: white;
590 -}
591 -td.diff-addedline {
592 - background: #cfc;
593 - font-size: smaller;
594 -}
595 -td.diff-deletedline {
596 - background: #ffa;
597 - font-size: smaller;
598 -}
599 -td.diff-context {
600 - background: #eee;
601 - font-size: smaller;
602 -}
603 -.diffchange {
604 - color: red;
605 - font-weight: bold;
606 - text-decoration: none;
607 -}
608 -
609 -table.diff td div {
610 - overflow: auto;
611 -}
612 -
613 -/*
614586 ** keep the whitespace in front of the ^=, hides rule from konqueror
615587 ** this is css3, the validator doesn't like it when validating as css2
616588 */
Index: trunk/phase3/skins/common/common.css
——@@ -110,27 +110,6 @@
111111 border:1px dashed #aaaaaa;
112112 }
113113
114 -table.diff { background:white; }
115 -td.diff-otitle { background:#cccccc; }
116 -td.diff-ntitle { background:#cccccc; }
117 -td.diff-addedline {
118 - background:#ccffcc;
119 - font-size: 94%;
120 -}
121 -td.diff-deletedline {
122 - background:#ffffaa;
123 - font-size: 94%;
124 -}
125 -td.diff-context {
126 - background:#eeeeee;
127 - font-size: 94%;
128 -}
129 -.diffchange {
130 - color: red;
131 - font-weight: bold;
132 - text-decoration: none;
133 -}
134 -
135114 img { border: none; }
136115 img.tex { vertical-align: middle; }
137116 span.texhtml { font-family: serif; }
Index: trunk/phase3/skins/common/diff.css
——@@ -0,0 +1,54 @@
 2+/*
 3+** Diff rendering
 4+*/
 5+table.diff, td.diff-otitle, td.diff-ntitle {
 6+ background-color: white;
 7+}
 8+td.diff-otitle,
 9+td.diff-ntitle {
 10+ text-align: center;
 11+}
 12+td.diff-marker {
 13+ text-align: right;
 14+}
 15+td.diff-addedline {
 16+ background: #cfc;
 17+ font-size: smaller;
 18+}
 19+td.diff-deletedline {
 20+ background: #ffa;
 21+ font-size: smaller;
 22+}
 23+td.diff-context {
 24+ background: #eee;
 25+ font-size: smaller;
 26+}
 27+.diffchange {
 28+ color: red;
 29+ font-weight: bold;
 30+ text-decoration: none;
 31+}
 32+
 33+table.diff {
 34+ border: none;
 35+ width: 98%;
 36+ border-spacing: 4px;
 37+
 38+ /* Fixed layout is required to ensure that cells containing long URLs
 39+ don't widen in Safari, Internet Explorer, or iCab */
 40+ table-layout: fixed;
 41+}
 42+table.diff td {
 43+ padding: 0;
 44+}
 45+table.diff col.diff-marker {
 46+ width: 2%;
 47+}
 48+table.diff col.diff-content {
 49+ width: 48%;
 50+}
 51+table.diff td div {
 52+ /* Scrollbars will be added for very wide cells
 53+ instead of text overflowing or widening */
 54+ overflow: auto;
 55+}
Property changes on: trunk/phase3/skins/common/diff.css
___________________________________________________________________
Name: svn:eol-style
156 + native
Index: trunk/phase3/includes/OutputPage.php
——@@ -74,6 +74,13 @@
7575 function addMeta( $name, $val ) { array_push( $this->mMetatags, array( $name, $val ) ); }
7676 function addKeyword( $text ) { array_push( $this->mKeywords, $text ); }
7777 function addScript( $script ) { $this->mScripts .= "\t\t".$script; }
 78+ function addStyle( $style ) {
 79+ global $wgStylePath;
 80+ $this->addLink(
 81+ array(
 82+ 'rel' => 'stylesheet',
 83+ 'href' => $wgStylePath . '/' . $style ) );
 84+ }
7885
7986 /**
8087 * Add a self-contained script tag with the given contents
Index: trunk/phase3/includes/DifferenceEngine.php
——@@ -96,6 +96,7 @@
9797 return;
9898 }
9999
 100+ $wgOut->addStyle( 'common/diff.css' );
100101 $wgOut->setArticleFlag( false );
101102 if ( ! $this->loadRevisionData() ) {
102103 $t = $this->mTitle->getPrefixedText() . " (Diff: {$this->mOldid}, {$this->mNewid})";
——@@ -488,10 +489,14 @@
489490 $ntitle = '<span class="history-deleted">'.$ntitle.'</span>';
490491 }
491492 $header = "
492 - <table border='0' width='98%' cellpadding='0' cellspacing='4' class='diff'>
 493+ <table class='diff'>
 494+ <col class='diff-marker' />
 495+ <col class='diff-content' />
 496+ <col class='diff-marker' />
 497+ <col class='diff-content' />
493498 <tr>
494 - <td colspan='2' width='50%' align='center' class='diff-otitle'>{$otitle}</td>
495 - <td colspan='2' width='50%' align='center' class='diff-ntitle'>{$ntitle}</td>
 499+ <td colspan='2' class='diff-otitle'>{$otitle}</td>
 500+ <td colspan='2' class='diff-ntitle'>{$ntitle}</td>
496501 </tr>
497502 ";
498503
——@@ -1738,18 +1743,26 @@
17391744
17401745 # HTML-escape parameter before calling this
17411746 function addedLine( $line ) {
1742 - return "<td>+</td><td class='diff-addedline'><div>{$line}</div></td>";
 1747+ return $this->wrapLine( '+', 'diff-addedline', $line );
17431748 }
17441749
17451750 # HTML-escape parameter before calling this
17461751 function deletedLine( $line ) {
1747 - return "<td>-</td><td class='diff-deletedline'><div>{$line}</div></td>";
 1752+ return $this->wrapLine( '-', 'diff-deletedline', $line );
17481753 }
17491754
17501755 # HTML-escape parameter before calling this
17511756 function contextLine( $line ) {
1752 - return "<td> </td><td class='diff-context'><div>{$line}</div></td>";
 1757+ return $this->wrapLine( ' ', 'diff-context', $line );
17531758 }
 1759+
 1760+ private function wrapLine( $marker, $class, $line ) {
 1761+ if( $line !== '' ) {
 1762+ // The <div> wrapper is needed for 'overflow: auto' style to scroll properly
 1763+ $line = "<div>$line</div>";
 1764+ }
 1765+ return "<td class='diff-marker'>$marker</td><td class='$class'>$line</td>";
 1766+ }
17541767
17551768 function emptyLine() {
17561769 return '<td colspan="2">&nbsp;</td>';
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1125,7 +1125,7 @@
11261126 * to ensure that client-side caches don't keep obsolete copies of global
11271127 * styles.
11281128 */
1129 -$wgStyleVersion = '68';
 1129+$wgStyleVersion = '69';
11301130
11311131
11321132 # Server-side caching:
Index: trunk/phase3/RELEASE-NOTES
——@@ -58,8 +58,10 @@
5959 * (bug 9417) Uploading new versions of images when using Postgres no longer
6060 throws warnings.
6161 * (bug 9908) Using tsearch2 with Postgres 8.1 no longer gives an error.
62 -* (bug 1438) Fix for diff table layout on very wide lines for Gecko and
63 - Opera-based browsers (incomplete, does not fix KHTML or MSIE)
 62+* (bug 1438) Fix for diff table layout on very wide lines.
 63+ Diff style rules have been broken out to common/diff.css,
 64+ and the dupes removed from the default skin files.
 65+ Skins can still override the default rules.
6466
6567
6668 == MediaWiki API changes since 1.10 ==
Index: trunk/extensions/SemanticMediaWiki/skins/mw-skins/ontoskin/handheld.css
——@@ -541,28 +541,6 @@
542542 border: 1px dashed #aaa;
543543 }
544544
545 -/*
546 -** Diff rendering
547 -*/
548 -table.diff, td.diff-otitle, td.diff-ntitle {
549 - background-color: white;
550 -}
551 -td.diff-addedline {
552 - background: #cfc;
553 - font-size: smaller;
554 -}
555 -td.diff-deletedline {
556 - background: #ffa;
557 - font-size: smaller;
558 -}
559 -td.diff-context {
560 - background: #eee;
561 - font-size: smaller;
562 -}
563 -.diffchange {
564 - color: red;
565 - font-weight: bold;
566 -}
567545
568546 /*
569547 ** keep the whitespace in front of the ^=, hides rule from konqueror
Index: trunk/extensions/sampleskin/main.css
——@@ -392,25 +392,6 @@
393393 background-color:#f9f9f9;
394394 border:1px dashed #aaaaaa;
395395 }
396 -/*
397 -** Diff rendering
398 -*/
399 -table.diff { background:white; }
400 -td.diff-otitle { background:#ffffff; }
401 -td.diff-ntitle { background:#ffffff; }
402 -td.diff-addedline {
403 - background:#ccffcc;
404 - font-size: smaller;
405 -}
406 -td.diff-deletedline {
407 - background:#ffffaa;
408 - font-size: smaller;
409 -}
410 -td.diff-context {
411 - background:#eeeeee;
412 - font-size: smaller;
413 -}
414 -span.diffchange { color: red; }
415396
416397 a.external { color: #3366bb; }
417398

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r22192* (bug 1438) Fix for diff table layout on very wide lines for Gecko and...brion21:24, 15 May 2007