r76326 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76325‎ | r76326 | r76327 >
Date:19:30, 8 November 2010
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
* Following id-naming, underscore to dash
* Wrapping mw.log in jQuery/mediaWiki
* Making mw-log-console fixed position instead of absolute (to no longer have it floating on top of the page after scrolling down)
* Added timestamp
* Changed inline styling to stylesheet
Modified paths:
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.log.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/Resources.php
@@ -323,6 +323,11 @@
324324 'debugScripts' => 'resources/mediawiki/mediawiki.log.js',
325325 'debugRaw' => false
326326 ) ),
 327+ 'mediawiki.util' => new ResourceLoaderFileModule( array(
 328+ 'scripts' => 'resources/mediawiki.util/mediawiki.util.js',
 329+ 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client' ),
 330+ 'debugScripts' => 'resources/mediawiki.util/mediawiki.util.test.js',
 331+ ) ),
327332 'mediawiki.advanced.rightclickedit' => new ResourceLoaderFileModule( array(
328333 'scripts' => 'resources/mediawiki.advanced/mediawiki.advanced.rightclickedit.js',
329334 ) ),
@@ -387,11 +392,6 @@
388393 'wa' => 'resources/mediawiki.language/languages/wa.js',
389394 ),
390395 ) ),
391 - 'mediawiki.util' => new ResourceLoaderFileModule( array(
392 - 'scripts' => 'resources/mediawiki.util/mediawiki.util.js',
393 - 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client' ),
394 - 'debugScripts' => 'resources/mediawiki.util/mediawiki.util.test.js',
395 - ) ),
396396
397397 /* mediawiki Legacy */
398398
Index: trunk/phase3/resources/mediawiki/mediawiki.log.js
@@ -2,52 +2,63 @@
33 * Implementation for mediaWiki.log stub
44 */
55
6 -/**
7 - * Log output to the console.
8 - *
9 - * In the case that the browser does not have a console available, one is created by appending a
10 - * <div> element to the bottom of the body and then appending a <div> element to that for each
11 - * message.
12 - *
13 - * @author Michael Dale <mdale@wikimedia.org>
14 - * @author Trevor Parscal <tparscal@wikimedia.org>
15 - * @param {string} string Message to output to console
16 - */
17 -mediaWiki.log = function( string ) {
18 - // Allow log messages to use a configured prefix
19 - if ( mw.config.exists( 'mw.log.prefix' ) ) {
20 - string = mw.config.get( 'mw.log.prefix' ) + string;
21 - }
22 - // Try to use an existing console
23 - if ( typeof window.console !== 'undefined' && typeof window.console.log == 'function' ) {
24 - window.console.log( string );
25 - } else {
26 - // Show a log box for console-less browsers
27 - var $log = jQuery( '#mw_log_console' );
28 - if ( !$log.length ) {
29 - $log = jQuery( '<div id="mw_log_console"></div>' )
30 - .css( {
31 - 'position': 'absolute',
32 - 'overflow': 'auto',
33 - 'z-index': 500,
34 - 'bottom': '0px',
35 - 'left': '0px',
36 - 'right': '0px',
37 - 'height': '150px',
38 - 'background-color': 'white',
39 - 'border-top': 'solid 1px #DDDDDD'
40 - } )
41 - .appendTo( jQuery( 'body' ) );
 6+(function ($, mw) {
 7+
 8+ /**
 9+ * Log output to the console.
 10+ *
 11+ * In the case that the browser does not have a console available, one is created by appending a
 12+ * <div> element to the bottom of the body and then appending a <div> element to that for each
 13+ * message.
 14+ *
 15+ * @author Michael Dale <mdale@wikimedia.org>
 16+ * @author Trevor Parscal <tparscal@wikimedia.org>
 17+ * @param {string} string Message to output to console
 18+ */
 19+ mediaWiki.log = function( string ) {
 20+ // Allow log messages to use a configured prefix
 21+ if ( mw.config.exists( 'mw.log.prefix' ) ) {
 22+ string = mw.config.get( 'mw.log.prefix' ) + '> ' + string;
4223 }
43 - $log.append(
44 - jQuery( '<div></div>' )
45 - .text( string )
46 - .css( {
47 - 'border-bottom': 'solid 1px #DDDDDD',
48 - 'font-size': 'small',
49 - 'font-family': 'monospace',
50 - 'padding': '0.125em 0.25em'
51 - } )
52 - );
53 - }
54 -};
 24+ // Try to use an existing console
 25+ if ( typeof window.console !== 'undefined' && typeof window.console.log !== 'function' ) {
 26+ window.console.log( string );
 27+ } else {
 28+ // Set timestamp
 29+ var d = new Date();
 30+ var time = ( d.getHours() < 10 ? '0' + d.getHours() : d.getHours() ) +
 31+ ':' + ( d.getMinutes() < 10 ? '0' + d.getMinutes() : d.getMinutes() ) +
 32+ ':' + ( d.getSeconds() < 10 ? '0' + d.getSeconds() : d.getSeconds() ) +
 33+ '.' + ( d.getMilliseconds() < 100 ? '0' + d.getMilliseconds() : d.getMilliseconds() );
 34+ // Show a log box for console-less browsers
 35+ var $log = $( '#mw-log-console' );
 36+ if ( !$log.length ) {
 37+ $log = $( '<div id="mw-log-console"></div>' )
 38+ .css( {
 39+ 'position': 'absolute',
 40+ 'overflow': 'auto',
 41+ 'z-index': 500,
 42+ 'bottom': '0px',
 43+ 'left': '0px',
 44+ 'right': '0px',
 45+ 'height': '150px',
 46+ 'background-color': 'white',
 47+ 'border-top': 'solid 2px #ADADAD'
 48+ } )
 49+ .appendTo( 'body' );
 50+ }
 51+ $log.append(
 52+ $( '<div></div>' )
 53+ .css( {
 54+ 'border-bottom': 'solid 1px #DDDDDD',
 55+ 'font-size': 'small',
 56+ 'font-family': 'monospace',
 57+ 'padding': '0.125em 0.25em'
 58+ } )
 59+ .text( string )
 60+ .append( '<span style="float:right">[' + time + ']</span>' )
 61+ );
 62+ }
 63+ };
 64+
 65+})(jQuery, mediaWiki);

Follow-up revisions

RevisionCommit summaryAuthorDate
r76379Reverting unintended change of r76326krinkle05:16, 9 November 2010
r76868Follow-up on r76326 - Fixing milliseconds zero padding for mw.logkrinkle00:41, 17 November 2010

Comments

#Comment by Brion VIBBER (talk | contribs)   03:42, 9 November 2010

The second condition in this if statement got flipped:

+		if ( typeof window.console !== 'undefined' && typeof window.console.log !== 'function' ) {

This breaks the ability to use an existing Firebug-style console; it needs to be switched back to == 'function'.

#Comment by Krinkle (talk | contribs)   05:16, 9 November 2010

Oops, I didn't intend to change that line at all. Fixed in r76379

#Comment by Catrope (talk | contribs)   21:41, 13 November 2010
+				 '.' + ( d.getMilliseconds() < 100 ? '0' + d.getMilliseconds() : d.getMilliseconds() );

Needs to account for single-digit ms numbers and prepend '00' to them.

Also, the stylesheet thing from the commit summary is nowhere to be found.

#Comment by Krinkle (talk | contribs)   01:01, 17 November 2010

Zero-padding fixed in r76868.

The stylesheet thing wasn't done. After an IRC discussion we said the overhead wasn't needed for the little gain, forgot to remove it from the COMMIT-message file before committing.

Status & tagging log