From bf1168b8cde7145ce4c85ba9bcb765494c288823 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Thu, 18 Sep 2014 16:49:08 +0200 Subject: [PATCH] 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 --- RELEASE-NOTES-1.24 | 6 ++ includes/AutoLoader.php | 1 - includes/DefaultSettings.php | 7 +- includes/installer/Installer.php | 5 -- .../ResourceLoaderLESSFunctions.php | 72 ------------------- .../src/mediawiki.less/mediawiki.mixins.less | 32 ++------- .../resourceloader/ResourceLoaderLESSTest.php | 29 -------- .../fixtures/001-embeddable.css | 9 --- .../fixtures/001-embeddable.less | 20 ------ 9 files changed, 16 insertions(+), 165 deletions(-) delete mode 100644 includes/resourceloader/ResourceLoaderLESSFunctions.php delete mode 100644 tests/phpunit/includes/resourceloader/ResourceLoaderLESSTest.php delete mode 100644 tests/phpunit/includes/resourceloader/fixtures/001-embeddable.css delete mode 100644 tests/phpunit/includes/resourceloader/fixtures/001-embeddable.less diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index ce083e084c..af0f15041a 100644 --- 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 d1229c4552..6b0daa1473 100644 --- 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 9ec47f2cef..9e57670842 100644 --- 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 f84ed0005d..8160e3d3dc 100644 --- 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 index 987b9025c2..0000000000 --- a/includes/resourceloader/ResourceLoaderLESSFunctions.php +++ /dev/null @@ -1,72 +0,0 @@ -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 8c288840db..c360e1f4a0 100644 --- a/resources/src/mediawiki.less/mediawiki.mixins.less +++ b/resources/src/mediawiki.less/mediawiki.mixins.less @@ -10,13 +10,8 @@ * See 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 index a3d73e54f3..0000000000 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderLESSTest.php +++ /dev/null @@ -1,29 +0,0 @@ -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 index b291c5e0c7..0000000000 --- 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 index 7018aa254d..0000000000 --- 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"); -} -- 2.20.1