Skip to content

fix #2434 - wrong calculation of offset#2437

Merged
SergioCrisostomo merged 2 commits into
mootools:masterfrom
rrelmy:master
Aug 10, 2015
Merged

fix #2434 - wrong calculation of offset#2437
SergioCrisostomo merged 2 commits into
mootools:masterfrom
rrelmy:master

Conversation

@rrelmy

@rrelmy rrelmy commented Oct 19, 2012

Copy link
Copy Markdown
Contributor

Fixes the bug described in #2434

Offsets are rounded correctly now

@rrelmy

rrelmy commented Mar 3, 2014

Copy link
Copy Markdown
Contributor Author

Does something block this commit to be merged?

@ibolmo

ibolmo commented Mar 3, 2014

Copy link
Copy Markdown
Member

Are you using sub pixels when setting the left and top?

@ibolmo

ibolmo commented Mar 3, 2014

Copy link
Copy Markdown
Member

Update the PR to use String.toFloat instead.

@rrelmy

rrelmy commented Mar 3, 2014

Copy link
Copy Markdown
Contributor Author

The problem mostly occurs on slideshows if you use an element centered with auto margins within an element of odd width or using percents in width/padding.

The pull request has been updated.

@ibolmo

ibolmo commented Mar 3, 2014

Copy link
Copy Markdown
Member

This is going to be on hold, until we get a spec to check for this. You're welcome to add one yourself, or wait about a week to get a hold of the pending PR that reduces the complexity to test MooTools.

@rrelmy

rrelmy commented Jul 15, 2015

Copy link
Copy Markdown
Contributor Author

I have rebased the previous commit and written a test spec.

PhantomJS version 1.9.8 used by the karma runner is based on a webkit version released around 2011 before subpixel alignment was added to webkit.
EDIT: I broke the builds again, tested locally against phantomjs v2 …

Is a test spec needed for this change? If the browser itself does not support subpixel alignment it is just fine, but I can't think of a way to test this usefully without breaking the PhantomJS build …

If something is missing please let me know.

@kentaromiura

Copy link
Copy Markdown
Member

LGTM but travis failed

@SergioCrisostomo

Copy link
Copy Markdown
Member

Travis fails on PhantomJS tests. We could add a bypass to phantom.js in the specs for this.

@rrelmy

rrelmy commented Jul 16, 2015

Copy link
Copy Markdown
Contributor Author

The test case has been modified to compare the result of getPosition with the output of getBoundingClientRect.
All tests pass now without testing for PhantomJS

SergioCrisostomo added a commit that referenced this pull request Aug 10, 2015
fix #2434 - wrong calculation of offset
@SergioCrisostomo SergioCrisostomo merged commit b1830c0 into mootools:master Aug 10, 2015
ickata pushed a commit to ickata/mootools-core that referenced this pull request Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants