r92580 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92579‎ | r92580 | r92581 >
Date:21:19, 19 July 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Fixes for URL expanding in CSSMin: adjust the offset correctly (this could've theoretically resulted in very strange bugs) and only call wfExpandUrl() if available (the file is in includes/libs so it should work outside of MediaWiki)
Modified paths:
  • /trunk/phase3/includes/libs/CSSMin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/libs/CSSMin.php
@@ -131,11 +131,18 @@
132132 // to absolute URLs but otherwise left alone
133133 if ( $match['file'][0] !== '' && $match['file'][0][0] === '/' ) {
134134 // Replace the file path with an expanded URL
135 - $source = substr_replace( $source, wfExpandUrl( $match['file'][0] ),
136 - $match['file'][1], strlen( $match['file'][0] )
137 - );
 135+ // ...but only if wfExpandUrl() is even available. This will not be the case if we're running outside of MW
 136+ $lengthIncrease = 0;
 137+ if ( function_exists( 'wfExpandUrl' ) ) {
 138+ $expanded = wfExpandUrl( $match['file'][0] );
 139+ $origLength = strlen( $match['file'][0] );
 140+ $lengthIncrease = strlen( $expanded ) - $origLength;
 141+ $source = substr_replace( $source, wfExpandUrl( $match['file'][0] ),
 142+ $match['file'][1], $origLength
 143+ );
 144+ }
138145 // Move the offset to the end of the match, leaving it alone
139 - $offset = $match[0][1] + strlen( $match[0][0] );
 146+ $offset = $match[0][1] + strlen( $match[0][0] ) + $lengthIncrease;
140147 continue;
141148 }
142149 // Shortcuts

Sign-offs

UserFlagDate
Nikerabbitinspected09:06, 20 July 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r94456Followup to r92580 and r93820: r92580 duplicated the call to wfExpandUrl(), a...catrope13:35, 14 August 2011
r94472MFT to REL1_18:...hashar19:43, 14 August 2011

Comments

#Comment by Catrope (talk | contribs)   13:36, 14 August 2011

Tagging for merge to 1.18 but not 1.17wmf1, because in the latter branch I've already merged r93820 and resolved the conflict caused by missing this revision.

Status & tagging log