r61961 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61960‎ | r61961 | r61962 >
Date:00:00, 4 February 2010
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Added some more filtering, as <p><br><p> was causing double lines in FF.
Modified paths:
  • /trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/js/plugins/jquery.wikiEditor.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php
@@ -72,7 +72,7 @@
7373 array( 'src' => 'js/plugins/jquery.namespaceSelect.js', 'version' => 1 ),
7474 array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 7 ),
7575 array( 'src' => 'js/plugins/jquery.textSelection.js', 'version' => 27 ),
76 - array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 100 ),
 76+ array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 101 ),
7777 array( 'src' => 'js/plugins/jquery.wikiEditor.highlight.js', 'version' => 29 ),
7878 array( 'src' => 'js/plugins/jquery.wikiEditor.toolbar.js', 'version' => 47 ),
7979 array( 'src' => 'js/plugins/jquery.wikiEditor.dialogs.js', 'version' => 12 ),
@@ -82,10 +82,10 @@
8383 array( 'src' => 'js/plugins/jquery.wikiEditor.publish.js', 'version' => 2 ),
8484 ),
8585 'combined' => array(
86 - array( 'src' => 'js/plugins.combined.js', 'version' => 217 ),
 86+ array( 'src' => 'js/plugins.combined.js', 'version' => 218 ),
8787 ),
8888 'minified' => array(
89 - array( 'src' => 'js/plugins.combined.min.js', 'version' => 217 ),
 89+ array( 'src' => 'js/plugins.combined.min.js', 'version' => 218 ),
9090 ),
9191 ),
9292 );
Index: trunk/extensions/UsabilityInitiative/js/plugins/jquery.wikiEditor.js
@@ -442,9 +442,11 @@
443443 html = html
444444 .replace( /\r?\n/g, "" ) // IE7 inserts newlines before block elements
445445 .replace( /&nbsp;/g, " " ) // We inserted these to prevent IE from collapsing spaces
 446+ .replace( /\<br[^\>]*\>\s*\<\/p\>/gi, '</p>' ) // Remove trailing <br> from <p>
 447+ .replace( /\<p[^\>]*\>\s*\<\/p\>/gi, '' ) // Collapse empty <p>
 448+ .replace( /\<\/p\>\s*\<p[^\>]*\>/gi, "\n" ) // Easy case for <p> conversion
446449 .replace( /\<br[^\>]*\>/gi, "\n" ) // <br> conversion
447 - .replace( /\<\/p\>\<p\>/gi, "\n" ) // Easy case for <p> conversion
448 - .replace( /\<\/p\>(\n*)\<p\>/gi, "$1\n" );
 450+ .replace( /\<\/p\>(\n*)\<p[^\>]*\>/gi, "$1\n" );
449451 // Save leading and trailing whitespace now and restore it later. IE eats it all, and even Firefox
450452 // won't leave everything alone
451453 var leading = html.match( /^\s*/ )[0];

Comments

#Comment by Catrope (talk | contribs)   14:58, 4 February 2010
+					.replace( /\<p[^\>]*\>\s*\<\/p\>/gi, '' ) // Collapse empty <p>

Empty Ps are actually legit in IE. Did you test this?

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:38, 4 February 2010

Why, of course I did. Although in IE

are indeed valid, this doesn't actually remove their ability to impart a \n character, it merely strips out the actual tags and their content.

#Comment by Catrope (talk | contribs)   20:23, 12 February 2010

The offending line was removed later.

Status & tagging log