* (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.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 310bd6e..e1a5854 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -58,8 +58,10 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
 * (bug 9417) Uploading new versions of images when using Postgres no longer 
   throws warnings.
 * (bug 9908) Using tsearch2 with Postgres 8.1 no longer gives an error.
-* (bug 1438) Fix for diff table layout on very wide lines for Gecko and
-  Opera-based browsers (incomplete, does not fix KHTML or MSIE)
+* (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.
 
 
 == MediaWiki API changes since 1.10 ==

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 39fda53..1361cda 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -1125,7 +1125,7 @@ $wgCacheEpoch = '20030516000000';
  * to ensure that client-side caches don't keep obsolete copies of global
  * styles.
  */
-$wgStyleVersion = '68';
+$wgStyleVersion = '69';
 
 
 # Server-side caching:

diff --git a/includes/DifferenceEngine.php b/includes/DifferenceEngine.php
index e8c8b2f..172e89e 100644 (file)
--- a/includes/DifferenceEngine.php
+++ b/includes/DifferenceEngine.php
@@ -96,6 +96,7 @@ CONTROL;
                        return;
                }
 
+               $wgOut->addStyle( 'common/diff.css' );
                $wgOut->setArticleFlag( false );
                if ( ! $this->loadRevisionData() ) {
                        $t = $this->mTitle->getPrefixedText() . " (Diff: {$this->mOldid}, {$this->mNewid})";
@@ -488,10 +489,14 @@ CONTROL;
                   $ntitle = '<span class="history-deleted">'.$ntitle.'</span>';
                }
                $header = "
-                       <table border='0' width='98%' cellpadding='0' cellspacing='4' class='diff'>
+                       <table class='diff'>
+                       <col class='diff-marker' />
+                       <col class='diff-content' />
+                       <col class='diff-marker' />
+                       <col class='diff-content' />
                        <tr>
-                               <td colspan='2' width='50%' align='center' class='diff-otitle'>{$otitle}</td>
-                               <td colspan='2' width='50%' align='center' class='diff-ntitle'>{$ntitle}</td>
+                               <td colspan='2' class='diff-otitle'>{$otitle}</td>
+                               <td colspan='2' class='diff-ntitle'>{$ntitle}</td>
                        </tr>
                ";
 
@@ -1738,17 +1743,25 @@ class TableDiffFormatter extends DiffFormatter
 
        # HTML-escape parameter before calling this
        function addedLine( $line ) {
-               return "<td>+</td><td class='diff-addedline'><div>{$line}</div></td>";
+               return $this->wrapLine( '+', 'diff-addedline', $line );
        }
 
        # HTML-escape parameter before calling this
        function deletedLine( $line ) {
-               return "<td>-</td><td class='diff-deletedline'><div>{$line}</div></td>";
+               return $this->wrapLine( '-', 'diff-deletedline', $line );
        }
 
        # HTML-escape parameter before calling this
        function contextLine( $line ) {
-               return "<td> </td><td class='diff-context'><div>{$line}</div></td>";
+               return $this->wrapLine( ' ', 'diff-context', $line );
+       }
+       
+       private function wrapLine( $marker, $class, $line ) {
+               if( $line !== '' ) {
+                       // The <div> wrapper is needed for 'overflow: auto' style to scroll properly
+                       $line = "<div>$line</div>";
+               }
+               return "<td class='diff-marker'>$marker</td><td class='$class'>$line</td>";
        }
 
        function emptyLine() {

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 76c2871..6a4f66d 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -74,6 +74,13 @@ class OutputPage {
        function addMeta( $name, $val ) { array_push( $this->mMetatags, array( $name, $val ) ); }
        function addKeyword( $text ) { array_push( $this->mKeywords, $text ); }
        function addScript( $script ) { $this->mScripts .= "\t\t".$script; }
+       function addStyle( $style ) {
+               global $wgStylePath;
+               $this->addLink(
+                               array(
+                                       'rel' => 'stylesheet',
+                                       'href' => $wgStylePath . '/' . $style ) );
+       }
 
        /**
         * Add a self-contained script tag with the given contents

diff --git a/skins/chick/main.css b/skins/chick/main.css
index 93a5f5f..997dd51 100644 (file)
--- a/skins/chick/main.css
+++ b/skins/chick/main.css
@@ -397,25 +397,6 @@ div.townBox dl dd {
     background-color:#f9f9f9;
     border:1px dashed #aaaaaa;
 }
-/*
-** Diff rendering
-*/
-table.diff { background:white; }
-td.diff-otitle { background:#ffffff; }
-td.diff-ntitle { background:#ffffff; }
-td.diff-addedline {
-    background:#ccffcc;
-    font-size: smaller;
-}
-td.diff-deletedline {
-    background:#ffffaa;
-    font-size: smaller;
-}
-td.diff-context {
-    background:#eeeeee;
-    font-size: smaller;
-}
-.diffchange { color: red; text-decoration: none; }
 
 a.external { color: #3366bb; }
 

diff --git a/skins/common/common.css b/skins/common/common.css
index 967a74e..25b716c 100644 (file)
--- a/skins/common/common.css
+++ b/skins/common/common.css
@@ -110,27 +110,6 @@ div.tleft {
     border:1px dashed #aaaaaa;
 }
 
-table.diff { background:white; }
-td.diff-otitle { background:#cccccc; }
-td.diff-ntitle { background:#cccccc; }
-td.diff-addedline {
-    background:#ccffcc;
-    font-size: 94%;
-}
-td.diff-deletedline {
-    background:#ffffaa;
-    font-size: 94%;
-}
-td.diff-context {
-    background:#eeeeee;
-    font-size: 94%;
-}
-.diffchange {
-       color: red;
-       font-weight: bold;
-       text-decoration: none;
-}
-
 img { border: none; }
 img.tex { vertical-align: middle; }
 span.texhtml { font-family: serif; }

diff --git a/skins/common/diff.css b/skins/common/diff.css
new file mode 100644 (file)
index 0000000..e2feefd
--- /dev/null
+++ b/skins/common/diff.css
@@ -0,0 +1,54 @@
+/*
+** Diff rendering
+*/
+table.diff, td.diff-otitle, td.diff-ntitle {
+       background-color: white;
+}
+td.diff-otitle,
+td.diff-ntitle {
+       text-align: center;
+}
+td.diff-marker {
+       text-align: right;
+}
+td.diff-addedline {
+       background: #cfc;
+       font-size: smaller;
+}
+td.diff-deletedline {
+       background: #ffa;
+       font-size: smaller;
+}
+td.diff-context {
+       background: #eee;
+       font-size: smaller;
+}
+.diffchange {
+       color: red;
+       font-weight: bold;
+       text-decoration: none;
+}
+
+table.diff {
+       border: none;
+       width: 98%;
+       border-spacing: 4px;
+       
+       /* Fixed layout is required to ensure that cells containing long URLs
+          don't widen in Safari, Internet Explorer, or iCab */
+       table-layout: fixed;
+}
+table.diff td {
+       padding: 0;
+}
+table.diff col.diff-marker {
+       width: 2%;
+}
+table.diff col.diff-content {
+       width: 48%;
+}
+table.diff td div {
+       /* Scrollbars will be added for very wide cells
+          instead of text overflowing or widening */
+       overflow: auto;
+}

diff --git a/skins/monobook/handheld.css b/skins/monobook/handheld.css
index d3ef7b6..5322bab 100644 (file)
--- a/skins/monobook/handheld.css
+++ b/skins/monobook/handheld.css
@@ -541,29 +541,6 @@ div.townBox dl dd {
        border: 1px dashed #aaa;
 }
 
-/*
-** Diff rendering
-*/
-table.diff, td.diff-otitle, td.diff-ntitle {
-       background-color: white;
-}
-td.diff-addedline {
-       background: #cfc;
-       font-size: smaller;
-}
-td.diff-deletedline {
-       background: #ffa;
-       font-size: smaller;
-}
-td.diff-context {
-       background: #eee;
-       font-size: smaller;
-}
-.diffchange {
-       color: red;
-       font-weight: bold;
-}
-
 /*
 ** keep the whitespace in front of the ^=, hides rule from konqueror
 ** this is css3, the validator doesn't like it when validating as css2

diff --git a/skins/monobook/main.css b/skins/monobook/main.css
index 25a6f34..3198330 100644 (file)
--- a/skins/monobook/main.css
+++ b/skins/monobook/main.css
@@ -581,34 +581,6 @@ div.townBox dl dd {
        border: 1px dashed #aaa;
 }
 
-/*
-** Diff rendering
-*/
-table.diff, td.diff-otitle, td.diff-ntitle {
-       background-color: white;
-}
-td.diff-addedline {
-       background: #cfc;
-       font-size: smaller;
-}
-td.diff-deletedline {
-       background: #ffa;
-       font-size: smaller;
-}
-td.diff-context {
-       background: #eee;
-       font-size: smaller;
-}
-.diffchange {
-       color: red;
-       font-weight: bold;
-       text-decoration: none;
-}
-
-table.diff td div {
-       overflow: auto;
-}
-
 /*
 ** keep the whitespace in front of the ^=, hides rule from konqueror
 ** this is css3, the validator doesn't like it when validating as css2