r53143 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r53142‎ | r53143 | r53144 >
Date:18:52, 12 July 2009
Author:simetrical
Status:ok
Tags:
Comment:
Do not load old CSS fixes for new browsers

It's bad form to load a fix for "version X and later". That means that
if the browser vendor fixes the underlying bug, they'll be served
incorrect markup and break. On the other hand, if you check for just
"version X", then if they fix the bug they'll be fine, but if they
*don't* fix the bug they'll break, which is as it should be. :)

Specifically, I disabled loading RTL fixes for Opera 9.6: my testing
shows that they worsen display, don't improve it. All other fixes were
already not being loaded for browsers later than they should have been.
(Other than the five-year-old KHTML fix I removed outright in r53141,
which was loading for modern-day Safari and Chrome despite the
underlying bug very possibly having been fixed before WebKit even
existed . . .)

While I was at it, I made the variable names a bit saner. I kept the
old weird ones (none are clear on which versions they include) for
backward compatibility in case scripts were relying on them. Except
is_ff2_, which was so atrociously named that I had to put it out of its
misery. For the others, I added new xxx_bugs variables to make it clear
that's all they were tracking.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/wikibits.js
——@@ -12,7 +12,7 @@
1313 }
1414 // For accesskeys; note that FF3+ is included here!
1515 var is_ff2 = /firefox\/[2-9]|minefield\/3/.test( clientPC );
16 -var is_ff2_ = /firefox\/2/.test( clientPC );
 16+var ff2_bugs = /firefox\/2/.test( clientPC );
1717 // These aren't used here, but some custom scripts rely on them
1818 var is_ff2_win = is_ff2 && clientPC.indexOf('windows') != -1;
1919 var is_ff2_x11 = is_ff2 && clientPC.indexOf('x11') != -1;
——@@ -20,7 +20,10 @@
2121 var is_opera = true;
2222 var is_opera_preseven = window.opera && !document.childNodes;
2323 var is_opera_seven = window.opera && document.childNodes;
24 - var is_opera_95 = /opera\/(9.[5-9]|[1-9][0-9])/.test( clientPC );
 24+ var is_opera_95 = /opera\/(9\.[5-9]|[1-9][0-9])/.test( clientPC );
 25+ var opera6_bugs = is_opera_preseven;
 26+ var opera7_bugs = is_opera_seven && !is_opera_95;
 27+ var opera95_bugs = /opera\/(9\.5)/.test( clientPC );
2528 }
2629
2730 // Global external objects used by this script.
——@@ -93,13 +96,15 @@
9497
9598 // special stylesheet links
9699 if (typeof stylepath != 'undefined' && typeof skin != 'undefined') {
97 - if (is_opera_preseven) {
 100+ // FIXME: This tries to load the stylesheets even for skins where they
 101+ // don't exist, i.e., everything but Monobook.
 102+ if (opera6_bugs) {
98103 importStylesheetURI(stylepath+'/'+skin+'/Opera6Fixes.css');
99 - } else if (is_opera_seven && !is_opera_95) {
 104+ } else if (opera7_bugs) {
100105 importStylesheetURI(stylepath+'/'+skin+'/Opera7Fixes.css');
101 - } else if (is_opera_95) {
 106+ } else if (opera95_bugs) {
102107 importStylesheetURI(stylepath+'/'+skin+'/Opera9Fixes.css');
103 - } else if (is_ff2_) {
 108+ } else if (ff2_bugs) {
104109 importStylesheetURI(stylepath+'/'+skin+'/FF2Fixes.css');
105110 }
106111 }
Index: trunk/phase3/RELEASE-NOTES
——@@ -247,6 +247,7 @@
248248 * (bug 19442) Show/hide options on watchlist only work once
249249 * (bug 19602) PubMed Magic links now use updated NIH url
250250 * (bug 19637) externallinks have links to self
 251+* Don't load Opera 9.5 RTL fixes for Opera 9.6
251252
252253 == API changes in 1.16 ==
253254

Follow-up revisions

RevisionCommit summaryAuthorDate
r53371Escape . in regex...simetrical18:18, 16 July 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r53141Remove KHTMLFixes.css...simetrical18:36, 12 July 2009

Status & tagging log