Last modified: 2013-02-07 16:31:28 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 T45451, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 43451 - Investigate/fix ParserFunctions "Recent changes seem to have broken mod behavior for a lot of users. Notably, it broke the coordinate templates on a bunch of wikis."
Investigate/fix ParserFunctions "Recent changes seem to have broken mod behav...
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ParserFunctions (Other open bugs)
unspecified
All All
: Highest major (vote)
: ---
Assigned To: Brad Jorsch
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-27 00:57 UTC by Sam Reed (reedy)
Modified: 2013-02-07 16:31 UTC (History)
4 users (show)

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


Attachments

Description Sam Reed (reedy) 2012-12-27 00:57:28 UTC
From https://gerrit.wikimedia.org/r/#/c/39490/

"Recent changes seem to have broken mod behavior for a lot of users. Notably, it broke the coordinate templates on a bunch of wikis."

This is likely from a mix of:
https://gerrit.wikimedia.org/r/#/c/37368/ and https://gerrit.wikimedia.org/r/#/c/38278/

Does anyone have any examples of what is "broken"? If so, they should be added to the PHPUnit tests for future regression prevention. If no one does, it might be easiest just to deploy it again, and wait for the complaints... ;D


I'm guessing the main line at fault is:

-                               $stack[] = $left % $right;
+                               $stack[] = fmod( (float)$left, (float)$right );

which then became:

-                               $stack[] = fmod( (float)$left, (float)$right );
+                               $stack[] = fmod( (double)$left, (double)$right );

So the deployed code should have been:
$stack[] = fmod( (double)$left, (double)$right );
Comment 1 Brad Jorsch 2012-12-28 21:28:50 UTC
Gerrit change #37368 change did change the behavior of {{#expr: X mod Y }}. The old code,

 $stack[] = $left % $right;

is more or less equivalent to

 $stack[] = fmod( floor( (float)$left ), floor( (float)$right ) );

rather than to

 $stack[] = fmod( (float)$left, (float)$right );

Note that PHP doesn't have different precision floating point types (according to http://php.net/manual/en/language.types.float.php), so (float)$x and (double)$x are equivalent and therefore Gerrit change #38278 was a no-op.

While this change in behavior was requested in bug 6068, it seems that many templates are relying on the rounding behavior of the old implementation.

If we really want to solve bug 6068, it might be best to keep "mod" with the old rounding behavior and to introduce a new "fmod" operator. Change I6114c6e7 will do that.
Comment 2 Sam Reed (reedy) 2012-12-28 21:49:44 UTC
(In reply to comment #1)
> If we really want to solve bug 6068, it might be best to keep "mod" with the
> old rounding behavior and to introduce a new "fmod" operator. Change
> I6114c6e7
> will do that.


I came across https://www.mediawiki.org/wiki/Extension:ParserFunctions/Extended when looking into this (which has an fmod too)

I wonder if we should pull some other functions from it into ParserFunctions itself...
'Further it includes the PHP functions abs, floor, ceil, fmod, and sqrt, and a function idiv which applies conversion to an integer by the PHP function "(int)" (rounding towards zero) to the quotient of the arguments.'
Comment 3 Brad Jorsch 2012-12-28 21:58:49 UTC
We already have abs, floor, and ceil. sqrt wouldn't hurt, but see bug 6068 for far too much ... "discussion" about idiv.
Comment 4 Sam Reed (reedy) 2012-12-31 17:46:33 UTC
Merged

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


Navigation
Links