Last modified: 2013-03-26 13:00:37 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 38294 - jquery.articleFeedbackv5.special styles loaded incorrectly in RTL languages
jquery.articleFeedbackv5.special styles loaded incorrectly in RTL languages
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ArticleFeedbackv5 (Other open bugs)
unspecified
All All
: High major (vote)
: ---
Assigned To: Matthias Mullie
: i18n
: 39694 (view as bug list)
Depends on:
Blocks: rtl
  Show dependency treegraph
 
Reported: 2012-07-11 03:49 UTC by Robin Pepermans (SPQRobin)
Modified: 2013-03-26 13:00 UTC (History)
6 users (show)

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


Attachments

Description Robin Pepermans (SPQRobin) 2012-07-11 03:49:51 UTC
Compare https://en.wikipedia.org/wiki/Special:ArticleFeedbackv5/Golden-crowned_Sparrow to https://en.wikipedia.org/wiki/Special:ArticleFeedbackv5/Golden-crowned_Sparrow?uselang=ar or https://en.wikipedia.org/wiki/Special:ArticleFeedbackv5/Golden-crowned_Sparrow?uselang=he

For RTL languages, some CSS seem to not load, e.g. for the three links at the right (= supposed to be left for RTL languages).
Comment 1 Derk-Jan Hartman 2012-07-12 18:39:01 UTC
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
Comment 2 Robin Pepermans (SPQRobin) 2012-07-12 20:10:46 UTC
(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
Comment 3 Derk-Jan Hartman 2012-07-14 17:09:05 UTC
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.
Comment 4 Robin Pepermans (SPQRobin) 2012-08-27 19:42:58 UTC
*** Bug 39694 has been marked as a duplicate of this bug. ***
Comment 5 Roan Kattouw 2012-08-28 19:25:35 UTC
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.
Comment 6 Roan Kattouw 2012-08-28 19:37:55 UTC
Workaround submitted in https://gerrit.wikimedia.org/r/#/c/21775/
Comment 7 Roan Kattouw 2012-08-28 20:14:22 UTC
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
Comment 8 Tim Starling 2012-08-28 23:43:58 UTC
How long does CSSJanus take to run on that file, with a sufficient backtrack limit?
Comment 9 Roan Kattouw 2012-08-29 23:22:46 UTC
(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.
Comment 10 Tim Starling 2012-08-30 03:42:12 UTC
(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.
Comment 11 Roan Kattouw 2012-08-30 05:16:43 UTC
(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:).

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


Navigation
Links