Wikimedia Bugzilla is closed!

Point of extra interest. DOES add the css in debug mode.

https://en.wikipedia.org/wiki/Special:ArticleFeedbackv5/Golden-crowned_Sparrow?uselang=ar&debug=true

(In reply to comment #1)
> Point of extra interest. DOES add the css in debug mode.
> 
> https://en.wikipedia.org/wiki/Special:ArticleFeedbackv5/Golden-crowned_Sparrow?uselang=ar&debug=true

In general, debug mode mostly behaves like LTR regardless of the interface being LTR or RTL; see the note at https://www.mediawiki.org/wiki/Directionality_support#ResourceLoader

http://bits.wikimedia.org/en.wikipedia.org/load.php?debug=true&lang=en&modules=jquery.articleFeedbackv5.special&only=styles&skin=monobook&*

http://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=he&modules=jquery.articleFeedbackv5.special&only=styles&skin=vector&*

http://bits.wikimedia.org/en.wikipedia.org/load.php?debug=true&lang=ar&modules=jquery.articleFeedbackv5.special&only=styles&skin=vector&*

In he or ar mode, the entire css file of jquery.articleFeedbackv5.special.css is not being included by load.php (dashboard css has no issues, and switching to en. works fine as well).

I've gone over it multiple times now, i'm not sure what the problem is, seems like something is going wrong during flipping, but the same load.php query seems to work just fine on my local install.

*** Bug 39694 has been marked as a duplicate of this bug. ***

This seems to be because of unhelpful behavior in preg_replace_callback(), which returns null on error. And for some reason, the following is triggering an error and causing preg_replace_callback() to return null, even though preg_match() (and, not shown here, preg_match_all()) don't have a problem with the given regex and agree it doesn't match the string.

> $style = file_get_contents('/usr/local/apache/common/php-1.20wmf10/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css')

> $regex = '/(background(?:-position)?\s*:\s*[^%]*?)(-?(?:[0-9]*\.[0-9]+|[0-9]+))(%\s*(?:(?:[0-9]*\.[0-9]+|[0-9]+)(?:\s*(?:em|ex|px|cm|mm|in|pt|pc|deg|rad|grad|ms|s|hz|khz|%)|-?(?:[_a-z]|[\200-\377]|(?:(?:(?:\[0-9a-f]{1,6})(?:\r\n|\s)?)|\[^\r\n\f0-9a-f]))(?:[_a-z0-9-]|[\200-\377]|(?:(?:(?:\[0-9a-f]{1,6})(?:\r\n|\s)?)|\[^\r\n\f0-9a-f]))*)?|-?(?:[_a-z]|[\200-\377]|(?:(?:(?:\[0-9a-f]{1,6})(?:\r\n|\s)?)|\[^\r\n\f0-9a-f]))(?:[_a-z0-9-]|[\200-\377]|(?:(?:(?:\[0-9a-f]{1,6})(?:\r\n|\s)?)|\[^\r\n\f0-9a-f]))*))/';

> echo preg_match( $regex, $style )
0
> var_dump(preg_replace_callback($regex, function($matches) { return $matches[1] . (100 - $matches[2]) . $matches[3]; }, $style ))
NULL

Of course there's no way to find out what the error is, because it's not actually returned :S . Will fix the code to handle a null return value correctly.

Workaround submitted in https://gerrit.wikimedia.org/r/#/c/21775/

Aaron did some research and discovered this is because we're hitting PCRE's backtrack limit. PCRE's default backtrack limit is 1M, but PHP's default settings reduced it to 100k until PHP 5.3.7 (we run 5.3.2). Submitted a fix for this in puppet in https://gerrit.wikimedia.org/r/21824

How long does CSSJanus take to run on that file, with a sufficient backtrack limit?

(In reply to comment #8)
> How long does CSSJanus take to run on that file, with a sufficient backtrack
> limit?
On fenari:

> $style = file_get_contents('/usr/local/apache/common/php-1.20wmf10/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css')

> ini_set('pcre.backtrack_limit', 1e6)

> $start = microtime(true); for ( $i = 0; $i < 1000; $i++ ) { CSSJanus::transform($style, true, false ); } $end = microtime(true); echo $end-$start;
54.10337305069

So 54ms.

(In reply to comment #9)
> So 54ms.

OK, deployed. Usually if a regex requires very deep backtracking, it means it's O(N^2), so it could probably use some optimisation work. CSSJanus is only 300 lines of code, someone should just rewrite it as a proper parser.

(In reply to comment #10)
> (In reply to comment #9)
> > So 54ms.
> 
> OK, deployed. Usually if a regex requires very deep backtracking, it means it's
> O(N^2), so it could probably use some optimisation work. CSSJanus is only 300
> lines of code, someone should just rewrite it as a proper parser.
Yeah we need to do that anyway, because a pile of regexes isn't gonna be smart enough to deal with quoted strings with arbitrary text (which are legitimate in CSS because CSS3 added the abomination that is content:).