Fix CSSJanus flipping in LESS mixins and remove broken custom LESS functions

Custom LESS functions are problematic for us for a number of reasons,
as outlined by Timo on bug 67368. We should get rid of them.

The only use case was implementing CSSMin data: URI embedding in LESS,
which used to be impossible due to lessc not preserving comments (bug
54673). However, thanks to new syntax added in f3779e06 we can insert
the annotations in such a way that the compiler won't mess with them.
The same technique is used in OOjs UI since 584ed144.

The LESS-function-based embedding implementation also meant that we
were unable to flip images for RTL (bug 66091 and friends: bug 66773,
bug 68326). The annotation one doesn't have this limitation.

Bug: 67368
Bug: 66091
Bug: 66773
Bug: 68326
Change-Id: I3062346ed63272a1c22b5df27b4cc1de2a699d9a

diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24
index ce083e0..af0f150 100644 (file)
--- a/RELEASE-NOTES-1.24
+++ b/RELEASE-NOTES-1.24
@@ -70,6 +70,7 @@ production.
 * $wgCanonicalLanguageLinks has been removed. Per Google recommendations, we
   will not send a rel=canonical pointing to a variant-neutral page, however
   we will send rel=alternate.
+* $wgResourceLoaderLESSFunctions has been deprecated and will be removed in the future.
 
 === New features in 1.24 ===
 * Added new hook WatchlistEditorBeforeFormRender, allowing subscribers to
@@ -226,6 +227,8 @@ production.
 * (bug 69249) wfBaseConvert() now works around PHP Bug #50175 when using GMP.
 * (bug 57909) URLs in the externallinks table will no longer have certain
   characters decoded in the query string.
+* (bug 67368) LESS mixins like .background-image() correctly flip image
+  references for RTL stylesheets now.
 
 === Action API changes in 1.24 ===
 * action=parse API now supports prop=modules, which provides the list of
@@ -486,6 +489,8 @@ changes to languages because of Bugzilla reports.
 * In Linker.php, link(), linkText() and makeBrokenImageLinkObj() now display
   warnings if their first parameter is not a Title object. Also makeImageLink()
   now requires a Parser as its first parameter.
+* (bug 67368) LESS functions embed() and embeddable(), added in MediaWiki 1.23
+  and broken by design, have been removed. Use appropriate LESS mixins instead.
 
 ==== Renamed classes ====
 * CLDRPluralRuleConverter_Expression to CLDRPluralRuleConverterExpression
@@ -534,6 +539,7 @@ changes to languages because of Bugzilla reports.
 * RawPage - Use RawAction directly
 * StubContLang - Use Language::factory() instead
 * XMLReader2 - Use XMLReader directly
+* ResourceLoaderLESSFunctions - No longer in use, not intended for public usage
 
 == Compatibility ==
 

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index d1229c4..6b0daa1 100644 (file)
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -886,7 +886,6 @@ $wgAutoloadLocalClasses = array(
        'ResourceLoaderFileModule' => 'includes/resourceloader/ResourceLoaderFileModule.php',
        'ResourceLoaderFilePageModule' => 'includes/resourceloader/ResourceLoaderFilePageModule.php',
        'ResourceLoaderFilePath' => 'includes/resourceloader/ResourceLoaderFilePath.php',
-       'ResourceLoaderLESSFunctions' => 'includes/resourceloader/ResourceLoaderLESSFunctions.php',
        'ResourceLoaderModule' => 'includes/resourceloader/ResourceLoaderModule.php',
        'ResourceLoaderNoscriptModule' => 'includes/resourceloader/ResourceLoaderNoscriptModule.php',
        'ResourceLoaderSiteModule' => 'includes/resourceloader/ResourceLoaderSiteModule.php',

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 9ec47f2..9e57670 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3488,11 +3488,10 @@ $wgResourceLoaderLESSVars = array();
  * Changes to LESS functions do not trigger cache invalidation.
  *
  * @since 1.22
+ * @deprecated since 1.24 Questionable usefulness and problematic to support,
+ *     will be removed in the future.
  */
-$wgResourceLoaderLESSFunctions = array(
-       'embeddable' => 'ResourceLoaderLESSFunctions::embeddable',
-       'embed' => 'ResourceLoaderLESSFunctions::embed',
-);
+$wgResourceLoaderLESSFunctions = array();
 
 /**
  * Default import paths for LESS modules. LESS files referenced in @import

diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php
index f84ed00..8160e3d 100644 (file)
--- a/includes/installer/Installer.php
+++ b/includes/installer/Installer.php
@@ -1788,11 +1788,6 @@ abstract class Installer {
 
                // Some of the environment checks make shell requests, remove limits
                $GLOBALS['wgMaxShellMemory'] = 0;
-
-               // Don't bother embedding images into generated CSS, which is not cached
-               $GLOBALS['wgResourceLoaderLESSFunctions']['embeddable'] = function ( $frame, $less ) {
-                       return $less->toBool( false );
-               };
        }
 
        /**

diff --git a/includes/resourceloader/ResourceLoaderLESSFunctions.php b/includes/resourceloader/ResourceLoaderLESSFunctions.php
deleted file mode 100644 (file)
index 987b902..0000000
--- a/includes/resourceloader/ResourceLoaderLESSFunctions.php
+++ /dev/null
@@ -1,72 +0,0 @@
-<?php
-/**
- * PHP-provided functions for LESS; see docs for $wgResourceLoaderLESSFunctions
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
- *
- * @file
- */
-
-class ResourceLoaderLESSFunctions {
-       /**
-        * Check if an image file reference is suitable for embedding.
-        * An image is embeddable if it (a) exists, (b) has a suitable MIME-type,
-        * (c) does not exceed IE<9 size limit of 32kb. This is a LESS predicate
-        * function; it returns a LESS boolean value and can thus be used as a
-        * mixin guard.
-        *
-        * @par Example:
-        * @code
-        *   .background-image(@url) when(embeddable(@url)) {
-        *       background-image: url(@url) !ie;
-        *   }
-        * @endcode
-        * @param array $frame
-        * @param lessc $less
-        */
-       public static function embeddable( $frame, $less ) {
-               $base = pathinfo( $less->parser->sourceName, PATHINFO_DIRNAME );
-               $url = trim( $less->compileValue( $frame ), '"\'' );
-               $file = realpath( $base . '/' . $url );
-               return $less->toBool( $file
-                       && strpos( $url, '//' ) === false
-                       && filesize( $file ) < CSSMin::EMBED_SIZE_LIMIT
-                       && CSSMin::getMimeType( $file ) !== false );
-       }
-
-       /**
-        * Convert an image URI to a base64-encoded data URI.
-        *
-        * @par Example:
-        * @code
-        *   .fancy-button {
-        *       background-image: embed('../images/button-bg.png');
-        *   }
-        * @endcode
-        * @param array $frame
-        * @param lessc $less
-        * @return string
-        */
-       public static function embed( $frame, $less ) {
-               $base = pathinfo( $less->parser->sourceName, PATHINFO_DIRNAME );
-               $url = trim( $less->compileValue( $frame ), '"\'' );
-               $file = realpath( $base . '/' . $url );
-
-               $data = CSSMin::encodeImageAsDataURI( $file );
-               $less->addParsedFile( $file );
-               return CSSMin::buildUrlValue( $data );
-       }
-}

diff --git a/resources/src/mediawiki.less/mediawiki.mixins.less b/resources/src/mediawiki.less/mediawiki.mixins.less
index 8c28884..c360e1f 100644 (file)
--- a/resources/src/mediawiki.less/mediawiki.mixins.less
+++ b/resources/src/mediawiki.less/mediawiki.mixins.less
@@ -10,13 +10,8 @@
  * See <http://lesscss.org/#-mixins> for more information about how to write mixins.
  */
 
-.background-image(@url) when (embeddable(@url)) {
-       background-image: embed(@url);
-       background-image: url(@url)!ie;
-}
-
-.background-image(@url) when not (embeddable(@url)) {
-       background-image: url(@url);
+.background-image(@url) {
+       background-image: e('/* @embed */') url(@url);
 }
 
 .vertical-gradient(@startColor: gray, @endColor: white, @startPos: 0, @endPos: 100%) {
@@ -37,27 +32,14 @@
  * We do not embed the fallback image on the assumption that the gain for old browsers
  * is not worth the harm done to modern ones.
  */
-.background-image-svg(@svg, @fallback) when (embeddable(@svg)) {
+.background-image-svg(@svg, @fallback) {
        background-image: url(@fallback);
-       /* We don't need the !ie hack because this old IE uses the fallback already */
-       background-image: -webkit-linear-gradient(transparent, transparent), embed(@svg);
-       background-image: linear-gradient(transparent, transparent), embed(@svg);
-}
-
-.background-image-svg(@svg, @fallback) when not (embeddable(@svg)) {
-       background-image: url(@fallback);
-       background-image: -webkit-linear-gradient(transparent, transparent), url(@svg);
-       background-image: linear-gradient(transparent, transparent), url(@svg);
-}
-
-/* Caution: Does not support localisable images */
-.list-style-image(@url) when (embeddable(@url)) {
-       list-style-image: embed(@url);
-       list-style-image: url(@url)!ie;
+       background-image: -webkit-linear-gradient(transparent, transparent), e('/* @embed */') url(@svg);
+       background-image: linear-gradient(transparent, transparent), e('/* @embed */') url(@svg);
 }
 
-.list-style-image(@url) when not (embeddable(@url)) {
-       list-style-image: url(@url);
+.list-style-image(@url) {
+       list-style-image: e('/* @embed */') url(@url);
 }
 
 .transition(@value) {

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderLESSTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderLESSTest.php
deleted file mode 100644 (file)
index a3d73e5..0000000
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderLESSTest.php
+++ /dev/null
@@ -1,29 +0,0 @@
-<?php
-
-class ResourceLoaderLESSTest extends MediaWikiTestCase {
-       public static function lessProvider() {
-               $result = array();
-               foreach ( glob( __DIR__ . '/fixtures/*.less' ) as $file ) {
-                       $result[] = array( $file );
-               }
-
-               return $result;
-       }
-
-       /**
-        * @dataProvider lessProvider
-        */
-       public function testLessFile( $lessFile ) {
-               $cssFile = substr( $lessFile, 0, -4 ) . 'css';
-               if ( !file_exists( $cssFile ) ) {
-                       $this->fail( "No css file found to assert equal to $lessFile" );
-                       return;
-               }
-
-               $expect = file_get_contents( $cssFile );
-               $content = file_get_contents( $lessFile );
-               $result = ResourceLoader::getLessCompiler( RequestContext::getMain()->getConfig() )
-                       ->compile( $content, $lessFile );
-               $this->assertEquals( $expect, $result );
-       }
-}

diff --git a/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.css b/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.css
deleted file mode 100644 (file)
index b291c5e..0000000
--- a/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.css
+++ /dev/null
@@ -1,9 +0,0 @@
-.box {
-  content: not-embeddable;
-}
-.box {
-  content: embeddable;
-}
-.box {
-  content: embeddable;
-}

diff --git a/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.less b/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.less
deleted file mode 100644 (file)
index 7018aa2..0000000
--- a/tests/phpunit/includes/resourceloader/fixtures/001-embeddable.less
+++ /dev/null
@@ -1,20 +0,0 @@
-@base: '../fixtures/';
-
-.helper(@url) when (embeddable(@url)) {
-       content: embeddable;
-}
-.helper(@url) when not (embeddable(@url)) {
-       content: not-embeddable;
-}
-
-.box {
-       .helper('path/to/nonexistent/file');
-}
-
-.box {
-       .helper('001-embeddable.css');
-}
-
-.box {
-       .helper("@{base}001-embeddable.css");
-}