When processing:
url( 'foo.svg#anchors' )
getLocalFileReferences() would yield a file 'foo.svg#anchors'. It might
occur with upstream libraries, such as WikiFonts used by the Refreshed
skin.
When a path is found, strip the anchor entirely. Skip the case if that
results in an empty file.
remap() would properly skip the behavior "#default#', also skip url that
are solely an anchor such as '#other'.
Add a few test cases to cover CSSMin::getLocalFileReferences and cover
anchors usage in CSSMin::remap.
Bug: T115436
Change-Id: I1749ddc2b89021807f42d64131931ad7a99a7b43
In cases where we're operating on text data (and not binary data),
use e.g. "\u{00A0}" to refer directly to the Unicode character
'NO-BREAK SPACE' instead of "\xc2\xa0" to specify the bytes C2h A0h
(which correspond to the UTF-8 encoding of that character). This
makes it easier to look up those mysterious sequences, as not all
are as recognizable as the no-break space.
This is not enforced by PHP, but I think we should write those in
uppercase and zero-padded to at least four characters, like the
Unicode standard does.
Note that not all "\xNN" escapes can be automatically replaced:
* We can't use Unicode escapes for binary data that is not UTF-8
(e.g. in code converting from legacy encodings or testing the
handling of invalid UTF-8 byte sequences).
* '\xNN' escapes in regular expressions in single-quoted strings
are actually handled by PCRE and have to be dealt with carefully
(those regexps should probably be changed to use the /u modifier).
* "\xNN" referring to ASCII characters ("\x7F" and lower) should
probably be left as-is.
The replacements in this commit were done semi-manually by piping
the existing "\xNN" escapes through the following terrible Ruby
script I devised:
chars = eval('"' + ARGV[0] + '"').force_encoding('utf-8')
puts chars.split('').map{|char|
'\\u{' + char.ord.to_s(16).upcase.rjust(4, '0') + '}'
}.join('')
Change-Id: Idc3dee3a7fb5ebfaef395754d8859b18f1f8769a
This is called relatively often. Even small improvements might have an
impact.
I'm intentionally replacing method_exists with class_exists because the
old check looked like it was done for backwards compatibility (MediaWiki
before 1.27 did not contained the method), while in reality this code is
meant to run without MediaWiki. This is much better reflected with a
straight "if this class doesn't exist, there is no MediaWiki".
I'm intentionally using the …::class feature. Yes, this works, even if the
class is not there.
Change-Id: I7f250a7cb000105bb751f68f25c6cc1c44c8f221
The current editors draft from 23 April 2018 does not require to escape
the REPLACEMENT CHARACTER (U+FFFD) as code point anymore.
https://drafts.csswg.org/cssom/#serialize-a-string
If the character is NULL (U+0000), then the REPLACEMENT CHARACTER
(U+FFFD).
https://www.w3.org/TR/2016/WD-cssom-1-20160317/#serialize-a-string
If the character is NULL (U+0000), then the REPLACEMENT CHARACTER
(U+FFFD) escaped as code point.
Change-Id: Ia67e89b3c9561ca29e133d61a2eca8f3db306d8c
The empty string being matched causes an undefined array index
notice in production, seen from various random gadgets, but spiked
after a change in MonoBook from last week that introduced a
broken background-image rule with empty string as url.
In browsers, that is actually interpreted as valid and "expands"
to the current url and re-fetches as Accept:image/*, silly, but
still broken. The broken icon was fixed in MonoBook, but we still
need to avoid trying to remap empty string as url.
Two changes:
1. Fix regex used by remap() to not match empty string.
This was already fixed for the 'url()' case without the
optional quotes, but with quotes, it was being matched as
non-empty. This is now fixed by using '+' instead of '*'.
Added tests to confirm they produce output, and PHPUnit
is configured to also assert no Notices are emitted (which
it converts to fatal exceptions).
2. Fix processUrlMatch() as sanity check to throw if the key
is missing.
Bug: T191237
Change-Id: I0ada337b0b4ab73c80236367ff79c31bcd13aa7d
* Trim whitespaces after opening and before closing parentheses and
brackets.
* Ensure by test case that the invalid "url (" will not become the
valid "url(" by minification.
This change also prevents the parsing problem in Firefox 57.0
https://bugzilla.mozilla.org/1418152 which is fixed in Firefox 57.0.1.
Change-Id: I804477ba7c6363f0e964fc8c7c0bc74d2d4c1a0d
Removing unnecessary XML declaration prolog when embedding SVGs
as data URI and adding more complex elements & attributes to test.
Bug: T175318
Change-Id: Ibec8c2ab498554a6ad33b4420ae6258dcc80fcdf
Convert newlines and tabs to spaces (which can be unencoded),
and consolidate runs of multiple spaces into a single space.
Also remove any leading and trailing spaces that might result
(most files end in a newline, for example).
Follow-up to Iddc98332d. This was reverted as dependency for the
original issues causing patch, but works perfectly fine.
Bug: T175318
Change-Id: Ic25d73caeb42ba7a32ef304806d401047b5a1997
I can see that "parent::__construct" literally calls the parent
constructor. I can see that stuff preceeded by the keyword "protected"
is protected. I really (really) don't need comments explaining such.
Change-Id: I7458e714976a6acd3ba6a7c93fdc27d03903df83
Convert newlines and tabs to spaces (which can be unencoded),
and consolidate runs of multiple spaces into a single space.
Also remove any leading and trailing spaces that might result
(most files end in a newline, for example).
Bug: T175318
Change-Id: Ic66c6acb37079cae84dd80ab2d5f2c829cf2df96
When a URL (data: or otherwise) contains quotes, don't wrap it in
double quotes (") but in single quotes ('). This then allows
us to unencode double quotes (") in the data URI embedding of SVGs.
Bug: T175318
Change-Id: I3e7eab64e1c3e82066014fb594f82d786983ce90
Follows-up eac059c722. See also https://stackoverflow.com/a/39676272/319266.
This function was never deprecated. For a short time, the www.php.net
manual page for `mime_content_type` wrongly documented it as having been
deprecated in PHP 5.3, but this wasn't true, and it has been present in
every PHP version since PHP 4.3, including PHP 7 and HHVM 2.3+.
Between PHP 4.3.0-4.3.2 and PHP 5.0-5.3, the function would be absent
if the Mimemagic extension was not enabled at compile-time. However, while
mime_content_type was first introduced by the Mimemagic PHP ext, it is
backend by the Finfo extension since PHP 5.3.0.
Confirmed via https://3v4l.org/IQC1Q.
* CSSMin: Revert conditional use of finfo back to unconditional use
of mime_content_type.
* MimeAnalyzer: Replace conditional use of finfo with unconditional use
use of mime_content_type. Also remove the now-redundant 'else' branch.
The 'else' branch existed because this code was written at a time where
MediaWiki still supported PHP 4, of which some minor versions could
sometimes be compiled without this function.
Change-Id: Iee4a0b6f616a469bb779c40e386045f9c3200446
And auto-fix all errors.
The `<exclude-pattern>` stanzas are now included in the default ruleset
and don't need to be repeated.
Change-Id: I928af549dc88ac2c6cb82058f64c7c7f3111598a
The used phpcs has a bug, so the version 0.9.0 could not be enforced at the moment.
Will be fixed in next version, see T167168
Changed:
- Remove duplicate newline at end of file
- Add space between function and ( for closures
- and -> &&, or -> ||
Change-Id: I4172fb08861729bccd55aecbd07e029e2638d311
Previously they were often being cut short due to the url pattern
ending at the first single quote, double quote or closing parenthesis
regardless of which of those started the url match.
Running benchmarkCSSMin.php before and after the change doesn't seem
produce consistent improvement or regression. Repeated runs with count=100
with and without this change both have a median between 2.6ms and 2.9ms
using PHP 5.6, and between 2.6ms and 2.8ms using HHVM 3.12.
Bug: T60473
Change-Id: I6d6a077ad76588f3ed81b1901a26b7e56d2157ee
About 0.74% of load.php is spent in CSSMin::getMimeType/finfo_file.
We don't apply this to user-generated content of external urls,
only local files in version control. And in terms of correctness,
we don't support misleading file names, so it's not a problem that
a foo.png containing XML/SVG content would get the wrong mime-type
when base64 embedding it in CSS.
Change-Id: I17686467c897984117671098e94db8732446dc75
I was bored. What? Don't look at me that way.
I mostly targetted mixed tabs and spaces, but others were not spared.
Note that some of the whitespace changes are inside HTML output,
extended regexps or SQL snippets.
Change-Id: Ie206cc946459f6befcfc2d520e35ad3ea3c0f1e0
It's unreasonable to expect newbies to know that "bug 12345" means "Task T14345"
except where it doesn't, so let's just standardise on the real numbers.
Skipping jsminplus.php as those bug numbers aren't Wikimedia's, nor obviously
someone else's.
Change-Id: I9a2210e17852ee56f11282b980ac66d8c7a95671
It looks like there is something missing after the last statement
Also remove some other empty lines at begin of functions, ifs or loops
while at these files
Change-Id: Ib00b5cfd31ca4dcd0c32ce33754d3c80bae70641
The check for file existence before checking its MIME type was
building the path incorrectly, causing this piece of code to always
think the file does not exist. (The code to actually embed the file
was correct.)
This was not caught by the tests before of a bug in the test code,
which cancelled out this one.
Bug: T100958
Change-Id: I4469079f41d48052d0a02f98bb1ed02407abd54c
Follows-up 8f5cd11d82.
The old getLocalFileReferences() method is no longer used anywhere.
Remove it and rename getAllLocalFileReferences back to it.
Change-Id: I864258aad128ba9b54464c7bc854543f2937f977
Keep in CSSMin as-is for back-compat and to ensure library remains
independent of MediaWiki.
Moved down a few lines as there is no need to compute the md5 hash when we're
returning a data URI. Previously md5_file was called twice during module builds
(once for the fallback url, and another time when producing the embedded data uri).
Applied to logo in SkinModule as example. To be applied elsewhere as needed.
Without it, fallback is current behaviour (no cache invalidation).
Bug: T99096
Change-Id: I7f38bfc1bea5c241bc4f8ec4f4b640fd65f2c04f
ResourceLoaderEditToolbarModule is clearly the wrong place for something so
generic, so this method needs a new home. We can either introduce a new class
or find a suitable existing home. I think CSSMin is suitable. It has public
methods that do things that are closely related, like `encodeStringAsDataURI`
and `buildUrlValue`.
Change-Id: Icc6dfb8f47199e6188dd71948f4645baee085e51
Change I5a3cdeb7d53485f161ccf8133e76850cdf5b4579 introduced tests
for existence of all local files referenced in CSS. It's currently
failing changes even when the referenced files have been commented
out. This change ensures that commented-out files are not included
in the testing list.
Change-Id: I2ad74c13a9cf36e96a7d7451bf229856bcd18422
The way this is implemented is really dirty... but it found us
a few pre-existing bugs already (T111518, T111519, T111771).
I think it might be worth it.
* CSSMin: Add new method getAllLocalFileReferences() which skips the
file_exists() check.
* ResourceLoaderModule: Make use of it to track missing files too.
* ResourcesTest: Verify that the missing files are missing.
Change-Id: I5a3cdeb7d53485f161ccf8133e76850cdf5b4579
The content of these files is more stable than their mtimes, which change
every time we roll out a new branch. Because MD5 avalanches well[0], using
the first five hexadecimal digits is sufficient to ensure that the chance
of two successive versions colliding is improbably small (roughly one in a
million).
[0]: https://en.wikipedia.org/wiki/Avalanche_effect
Change-Id: I1bdf94c58786d2545311b238476b48217a5a60af
Call wfRemoveDotSegments() (if available) on relative local URLs, so that
relative dot segments are collapsed.
Change-Id: Icc1c23a2a7d96c869412965bd8c3730d3c3b3a53
Both the problem and solution are nearly identical to the ones presented in
I31d4556bb. Use a constant string as a comment placeholder, and differentiate
it from possible user input using \x7f, which is not valid CSS anyway[1].
[1]: http://www.w3.org/TR/CSS21/grammar.html
Change-Id: I3d3c2578d5e9c1fe52c02e87901cb08d03847ea7
* Make it actually check against the URI length, not file size.
Previous code could produce URIs that edged just over the limit.
* Replace $sizeLimit parameter with $ie8Compat, because that's the
only use case and the file size limit approach was flawed per above,
anyway.
* Remove CSSMin::EMBED_SIZE_LIMIT constant, which is not used anymore.
CSSMin::DATA_URI_SIZE_LIMIT still exists.
* Change code flow to avoid repetition.
* Split off new method, encodeStringAsDataURI(), that does actual
encoding rather than file handling.
Change-Id: I3704404ce831d7e0f6e3a65f23c76f313ad13238