r83336 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r83335‎ | r83336 | r83337 >
Date:22:26, 5 March 2011
Author:hashar
Status:resolved (Comments)
Tags:
Comment:
Use 'Courier new' for $wgShowDebug HTML output

'Courier new' is rendered with a constant size in both monobook
and vector skins. monospace is rendered too small under vector
with firefox/Linux.
Thus, this patch makes the font size consistent.
Modified paths:
  • /trunk/phase3/includes/Skin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Skin.php
——@@ -827,7 +827,7 @@
828828
829829 if ( $wgShowDebug ) {
830830 $listInternals = $this->formatDebugHTML( $out->mDebugtext );
831 - return "\n<hr />\n<strong>Debug data:</strong><ul style=\"font-family:monospace;\" id=\"mw-debug-html\">" .
 831+ return "\n<hr />\n<strong>Debug data:</strong><ul style=\"font-family:\'Courier New\';\" id=\"mw-debug-html\">" .
832832 $listInternals . "</ul>\n";
833833 }
834834

Follow-up revisions

RevisionCommit summaryAuthorDate
r83460Keep monospace as a fallback for HTML debugging...hashar16:36, 7 March 2011
r84798Follow-up r83336, r83460: abandon hardcoded styles altogether, and instead us...happy-melon13:27, 26 March 2011

Comments

#Comment by Catrope (talk | contribs)   22:43, 5 March 2011

You should use font-family: monospace, "Courier New" instead, I think. That's a hack we used to solve the other instances, at least.

#Comment by JanPaul123 (talk | contribs)   23:39, 5 March 2011

It should be the other way around: font-family: "Courier New", monospace (see [1])

#Comment by Catrope (talk | contribs)   23:43, 5 March 2011

No. You actually do want the behavior of monospace, you just need to add an existing font name after it to work around a bug in Firefox.

#Comment by Hashar (talk | contribs)   12:38, 6 March 2011

What is the point of adding 'monospace' when we force 'Courier New' already ?

#Comment by Reach Out to the Truth (talk | contribs)   16:26, 6 March 2011

Someone who doesn't have Courier New wouldn't be getting any monospace font at all then.

#Comment by Hashar (talk | contribs)   16:37, 7 March 2011

That is the safest way. Got it with r83460, resetting to 'new'.

#Comment by Happy-melon (talk | contribs)   14:08, 6 March 2011

This shouldn't be applied with inline styles, but should instead use CSS; probably by adding the #mw-debug-html selector to the existing list of items which are formatted in monospace in shared.css.

#Comment by Hashar (talk | contribs)   15:53, 6 March 2011

Since it is just for developers, is there any need to get it in a public / user stylesheet ?

#Comment by Hashar (talk | contribs)   12:56, 26 March 2011

Can someone please re review this developer only CSS tweak? I believe both r83336 and r83460 are fine :-b

#Comment by Happy-melon (talk | contribs)   13:28, 26 March 2011

I'll see you a review and raise you a follow-up :D

Status & tagging log