Wrong offset calculation because of floating point values #2434

Closed
rrelmy opened this Issue Oct 16, 2012 · 4 comments

3 participants

@rrelmy

MooTools position calculation does not work correct if the element is position outside the screen if the page is centered within an document with an odd size.

The bounds in the getOffsets method are not rounded correctly.
If the offsets are absolute the rounding error does not affect the result but with a negative number the rounding error results in a wrong result.

Result should always be -250
Positive
Real calculation: 31.5-281.5 = -250
"Rounded" calculation: 31-281 = -250

Negative
Real: (-79.5)-170.5 = -250
Rounded: (-79)-170 = -249

Only affects webkit and gecko, ie and opera do not return floating point values.

See my testcase with a possible solution
With original MooTools: http://www.rrelmy.ch/mootools-position-bug/
With fixed MooTools: http://www.rrelmy.ch/mootools-position-bug/?fixed

@arian
MooTools member

You're right, it should simply round the value... Can you send a pull request?

@rrelmy

Pull request sent.

Are there any tests to verify that I did not break something else in mootools?

@arian
MooTools member

Yes, you could try to run the tests: https://github.com/mootools/mootools-core/tree/master/Specs#running-the-tests-in-a-browser. I'm not really sure if there are (good) tests for this specific method.

@ibolmo
MooTools member

Closing this so we can discuss in the PR.

@ibolmo ibolmo closed this Mar 3, 2014
@rrelmy rrelmy added a commit that referenced this issue Mar 3, 2014
@rrelmy rrelmy fix #2434 - wrong calculation of offset dc1ad82
@SergioCrisostomo SergioCrisostomo added a commit to SergioCrisostomo/mootools-core that referenced this issue Jun 7, 2014
@rrelmy rrelmy fix #2434 - wrong calculation of offset ebbd591
@ickata ickata added a commit to ickata/mootools-core that referenced this issue Sep 1, 2015
@rrelmy rrelmy fix #2434 - wrong calculation of offset 77d01cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment