Last modified: 2014-06-26 23:43:49 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T37492, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 35492 - CSSMin::minify should not interfere value contents of string values
CSSMin::minify should not interfere value contents of string values
Status: NEW
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.17.x
All All
: Normal normal (vote)
: Future release
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-26 17:07 UTC by Krinkle
Modified: 2014-06-26 23:43 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Krinkle 2012-03-26 17:07:33 UTC
Looking at a syntaxhighlighter plugin somewhere, I found the following sample (simplified here):

pre.code-json {
    position: relative;
    padding: 12px 7px;
}
pre.code-json::after {
    content: '{ "JSON": "code" };';
    position: absolute;
    bottom: 2px;
    right: 2px;
    ....
}

(which positions a label over the <pre>)

The CSSMin:minify function[1] currently does not respect string property values (either single or double quotes) and causes the following bug:

> return CSSMin::minify( 'foo::after { content: \'{ "JSON": "code" };\'; }' );
foo::after{content:'{"JSON":"code" };'}

Note that the value changed:

> content: '{ "JSON": "code" };'
> content: '{"JSON":"code" };'

In this case it's just a little whitespace, but I can come up with other examples where characters are removed and what not.

[1] https://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/libs/CSSMin.php?revision=110557&view=markup#l210
Comment 1 Tim Landscheidt 2012-09-22 19:18:59 UTC
Gerrit change #24670.
Comment 2 Krinkle 2012-09-22 19:54:30 UTC
Re-opening. Bugs are marked fixed when they are fixed in the software. Gerrit merge requests are not in the software yet, they're proposals in in gerrit which, if and when they pass review, will be submitted to the repository.
Comment 3 Tim Landscheidt 2012-09-22 22:05:47 UTC
(In reply to comment #2)
> Re-opening. Bugs are marked fixed when they are fixed in the software. Gerrit
> merge requests are not in the software yet, they're proposals in in gerrit
> which, if and when they pass review, will be submitted to the repository.

Sorry; I even used a similar case in bug 17322 comment 12 as a bad example.
Comment 4 Mark A. Hershberger 2012-09-30 16:59:57 UTC
I'd like to say I'm going to ask for the patch to be merged to 1.20, but it looks like Tim Starling has some substantive objections.  Pushing to future release.
Comment 5 Krinkle 2012-09-30 17:25:37 UTC
Yeah, the bug has been around for almost 2 years and so far (though we know how to trigger it) we haven't hit the bug in any of the real plugins and extensions we've come across. It is only a matter of time before we will, though.
Comment 6 Dereckson 2012-12-26 18:42:47 UTC
Matma Rex commented this bug on Gerrit:

"Before doing it properly, couldn't we extract all the strings and replace them with unique identifiers (using regexes, of course), apply regex whitespace magic, and re-insert the strings? This should be easy to do, the regex for strings would just have to be crafted carefully to handle escape sequences. (I sort of have a feeling this comment should go on the bug instead. Eh.)"

[ Bug assigned to code submitter. ]
Comment 7 Dereckson 2012-12-26 18:43:09 UTC
[ Adding Matma Rex as cc ]
Comment 8 James Forrester 2014-06-26 23:43:49 UTC
Commit listed above was abandonned; resetting to "NEW".

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links