r69335 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r69334‎ | r69335 | r69336 >
Date:17:50, 14 July 2010
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Improves on the technique used in r65391 to fix the font-size issues in Firefox, Chrome and Safari caused by using "monospace" as a font-family. The old approach was to use "monospace, sans-serif", which worked but was reported to actually fall-back on the sans-serif variant. Some research has shown that using '"Courier New", monospace' resolves the issue on all browsers without risking falling back to proportionally spaced fonts.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/skins/vector/main-ltr.css (modified) (history)
  • /trunk/phase3/skins/vector/main-rtl.css (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1547,7 +1547,7 @@
15481548 * to ensure that client-side caches do not keep obsolete copies of global
15491549 * styles.
15501550 */
1551 -$wgStyleVersion = '295';
 1551+$wgStyleVersion = '296';
15521552
15531553 /**
15541554 * This will cache static pages for non-logged-in users to reduce
Index: trunk/phase3/skins/vector/main-ltr.css
——@@ -721,7 +721,11 @@
722722 font-style: italic;
723723 }*/
724724 pre, code, tt {
725 - font-family: monospace, sans-serif;
 725+ /*
 726+ * It's important for this rule to first reference an actual font name, some browsers will render the monospace text
 727+ * too small otherwise, namely Firefox, Chrome and Safari
 728+ */
 729+ font-family: "Courier New", monospace;
726730 }
727731 code {
728732 background-color: #f9f9f9;
Index: trunk/phase3/skins/vector/main-rtl.css
——@@ -721,7 +721,11 @@
722722 font-style: italic;
723723 }*/
724724 pre, code, tt {
725 - font-family: monospace, sans-serif;
 725+ /*
 726+ * It's important for this rule to first reference an actual font name, some browsers will render the monospace text
 727+ * too small otherwise, namely Firefox, Chrome and Safari
 728+ */
 729+ font-family: "Courier New", monospace;
726730 }
727731 code {
728732 background-color: #f9f9f9;

Follow-up revisions

RevisionCommit summaryAuthorDate
r69336By flipping '"Courier New", monospace' to 'monospace, "Courier New"', we can ...tparscal18:31, 14 July 2010
r697291.16wmf4: Merge Vector fixes from trunk: r65063, r68798, r69335, r69336, r693...catrope14:09, 22 July 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r65391Actually fix bug #20706 - Next time I will read more carefully and test more ...tparscal14:19, 21 April 2010

Comments

#Comment by Catrope (talk | contribs)   18:07, 14 July 2010

From reading the sources mentioned at the comments for r65391 I believe that font-family: monospace, monospace will also work and will not force Courier New if the user has selected a different monospace font.

#Comment by Catrope (talk | contribs)   18:21, 14 July 2010

...although, upon further reading, I see that that's not true in all browsers. What about something like font-family: monospace, "Courier New", monospace? Is there any cross-browser solution that does not rely on the font name Courier New?

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:24, 14 July 2010

Safari 4, I've verified this myself, but also - look at the table here -> http://www.undermyhat.org/blog/2009/09/css-font-family-monospace-renders-inconsistently-in-firefox-and-chrome/#overview

#Comment by Catrope (talk | contribs)   18:25, 14 July 2010

Yes, saw that, see the first sentence of my second comment. Just wondering if there's any font name-agnostic way of doing this in e.g. Safari 4 (and Opera 8.5).

Status & tagging log