This reverts most of commit
2d842f14250646475b5c2ffa2fe4f5a131f94236,
leaving only the test added in it, and reimplements the same
functionality better.
Instead of stripping /*@noflip*/ annotations in CSSJanus, which is
incompatible with other implementations that preserve it, extend
CSSMin to allow other CSS comments to be present before the
rule-global @embed annotation. (This required making the regex logic
in it even worse than it was, but it's actually slightly less terrible
than I expected it would be. Good thing we have tests!)
Bug: 69698
Change-Id: I58603ef64f7d7cdc6461b34721a4d6b15f15ad79
if ( $flip === 'flip' && $this->getLanguage()->isRTL() ) {
# If wanted, and the interface is right-to-left, flip the CSS
$style_css = CSSJanus::transform( $style_css, true, false );
- } else {
- $style_css = CSSJanus::nullTransform( $style_css );
}
$this->mInlineStyles .= Html::inlineStyle( $style_css ) . "\n";
}
$previewedCSS = $this->getRequest()->getText( 'wpTextbox1' );
if ( $this->getLanguage()->getDir() !== $wgContLang->getDir() ) {
$previewedCSS = CSSJanus::transform( $previewedCSS, true, false );
- } else {
- $previewedCSS = CSSJanus::nullTransform( $previewedCSS );
}
$otherTags .= Html::inlineStyle( $previewedCSS ) . "\n";
} else {
$css = $noFlipClass->detokenize( $css );
$css = $noFlipSingle->detokenize( $css );
- // Remove remaining /* @noflip */ annotations, they won't be needed anymore
- // and can interfere with other code (bug 69698).
- $css = self::nullTransform( $css );
-
- return $css;
- }
-
- /**
- * Remove @noflip annotations, but don't do any other transforms.
- * @param string $css stylesheet to transform
- * @return string Transformed stylesheet
- */
- public static function nullTransform( $css ) {
- $patt = self::$patterns['noflip_annotation'];
- $css = preg_replace( "/($patt)\\s*/i", '', $css );
-
return $css;
}
$remote = substr( $remote, 0, -1 );
}
- // Replace all comments by a placeholder so they will not interfere
- // with the remapping
- // Warning: This will also catch on anything looking like the start of
- // a comment between quotation marks (e.g. "foo /* bar").
+ // Replace all comments by a placeholder so they will not interfere with the remapping.
+ // Warning: This will also catch on anything looking like the start of a comment between
+ // quotation marks (e.g. "foo /* bar").
$comments = array();
$placeholder = uniqid( '', true );
$source = preg_replace_callback(
$pattern,
- function ( $matchOuter ) use ( $local, $remote, $embedData ) {
+ function ( $matchOuter ) use ( $local, $remote, $embedData, $placeholder ) {
$rule = $matchOuter[0];
- // Check for global @embed comment and remove it
+ // Check for global @embed comment and remove it. Allow other comments to be present
+ // before @embed (they have been replaced with placeholders at this point).
$embedAll = false;
- $rule = preg_replace( '/^(\s*)' . CSSMin::EMBED_REGEX . '\s*/', '$1', $rule, 1, $embedAll );
+ $rule = preg_replace( '/^((?:\s+|' . $placeholder . '(\d+)x)*)' . CSSMin::EMBED_REGEX . '\s*/', '$1', $rule, 1, $embedAll );
// Build two versions of current rule: with remapped URLs
// and with embedded data: URIs (where possible).
if ( $flip ) {
$style = CSSJanus::transform( $style, true, false );
- } else {
- $style = CSSJanus::nullTransform( $style );
}
$localDir = dirname( $localPath );
$remoteDir = dirname( $remotePath );
$style = implode( "\n", $rules );
if ( $this->getFlip( $context ) ) {
$style = CSSJanus::transform( $style, true, false );
- } else {
- $style = CSSJanus::nullTransform( $style );
}
return array( 'all' => $style );
}
}
if ( $this->getFlip( $context ) ) {
$style = CSSJanus::transform( $style, true, false );
- } else {
- $style = CSSJanus::nullTransform( $style );
}
$style = CSSMin::remap( $style, false, $this->getConfig()->get( 'ScriptPath' ), true );
if ( !isset( $styles[$media] ) ) {
+/* @noflip */
.unit-tests {
color: green;
border: 2px solid #eeeeee;
if ( $cssB ) {
$transformedA = CSSJanus::transform( $cssA );
- $this->assertEquals(
- $transformedA,
- str_replace( '/* @noflip */ ', '', $cssB ),
- 'Test A-B transformation'
- );
+ $this->assertEquals( $transformedA, $cssB, 'Test A-B transformation' );
$transformedB = CSSJanus::transform( $cssB );
- $this->assertEquals(
- $transformedB,
- str_replace( '/* @noflip */ ', '', $cssA ),
- 'Test B-A transformation'
- );
+ $this->assertEquals( $transformedB, $cssA, 'Test B-A transformation' );
} else {
// If no B version is provided, it means
// the output should equal the input (modulo @noflip annotations).
$transformedA = CSSJanus::transform( $cssA );
- $this->assertEquals(
- $transformedA,
- str_replace( '/* @noflip */ ', '', $cssA ),
- 'Nothing was flipped'
- );
+ $this->assertEquals( $transformedA, $cssA, 'Nothing was flipped' );
}
}
'foo { /* @embed */ background: url(red.gif); }',
"foo { background: url($red); background: url(http://localhost/w/red.gif?timestamp)!ie; }",
),
+ array(
+ 'Embedded file, other comments before the rule',
+ "foo { /* Bar. */ /* @embed */ background: url(red.gif); }",
+ "foo { /* Bar. */ background: url($red); /* Bar. */ background: url(http://localhost/w/red.gif?timestamp)!ie; }",
+ ),
array(
'Can not re-embed data: URIs',
"foo { /* @embed */ background: url($red); }",
$this->assertStringEqualsFile( $basePath . '/styles.css', $styles['all'] );
}
+ /**
+ * Strip @noflip annotations from CSS code.
+ * @param string $css
+ * @return string
+ */
+ private function stripNoflip( $css ) {
+ return str_replace( '/*@noflip*/ ', '', $css );
+ }
+
/**
* What happens when you mix @embed and @noflip?
* This really is an integration test, but oh well.
$contextLtr = self::getResourceLoaderContext( 'en' );
$contextRtl = self::getResourceLoaderContext( 'he' );
+ // Since we want to compare the effect of @noflip+@embed against the effect of just @embed, and
+ // the @noflip annotations are always preserved, we need to strip them first.
$this->assertEquals(
$expectedModule->getStyles( $contextLtr ),
- $testModule->getStyles( $contextLtr ),
+ $this->stripNoflip( $testModule->getStyles( $contextLtr ) ),
"/*@noflip*/ with /*@embed*/ gives correct results in LTR mode"
);
$this->assertEquals(
$expectedModule->getStyles( $contextLtr ),
- $testModule->getStyles( $contextRtl ),
+ $this->stripNoflip( $testModule->getStyles( $contextRtl ) ),
"/*@noflip*/ with /*@embed*/ gives correct results in RTL mode"
);
}