fixes fractions handling in ui.position.

[inukshuk]

As discussed in #6000 and in core #7730 it is better if ui position leaves the values returned by offset() untouched (i.e., no Math.round or parseInt -- if the browser reports fractions let the browser decide what to do with them).

[scottgonzalez]

Does this fix the test that's commented out directly below the test you wrote? Also, is position currently broken with latest jQuery without this fix?

[inukshuk]

I've added the markup for the consistency test back in. Basically, if you run the test now (i.e., with the patch and jQuery 1.5.1 that is inside the test environment), it will fail. However, if you replace parseInt with parseFloat in the jQuery file (those are the only two instances of parseInt that are left) that test passes, too. So basically, you want to get rid of the Math.round with the next core release that uses parseFloat instead of parseInt.

Right now, with the current core release and using Math.round, position works fine most of the time but can lead to off-by-one pixel errors and leads to errors if there are sub-pixel positions involved (for example, the test I added may not work using the current position and latest jQuery).

[scottgonzalez]

Thanks, we'll probably wait till we update jQuery (which we should do soon) to land this.

[inukshuk]

Sounds good. Meanwhile, I've uncommented the #5280 test as it is supposed to work with the updated jQuery.

[inukshuk]

Any chance we can get this merged soon? If there are any issues left I'd gladly work on it; we've been patching ui for months and haven't noticed any problems --- it would be great if patching weren't necessary any longer ;-)

Thanks!

[scottgonzalez]

We can land this in master, but I'm not sure what to do in 1-8-stable where we support 1.3.2 - current. Any ideas for a clean way to detect whether we should be rounding?

[inukshuk]

It isn't pretty, but the only reasonable test I can think of is the version number: for jQuery below 1.6 we know that offset() will read CSS values using parseInt, therefore we could justify rounding the values in order to get consistent values on repeated calls. For 1.6 and later offset() should return fractions if they are set in CSS so it is OK to leave the values as they are. Does that sound reasonable?

[scottgonzalez]

Can we create an element, set the offset to have fractions and then use jQuery to get the offset? Does that give us the information we need?

[inukshuk]

Yes that may work. I could try that if you like and adapt the test case accordingly.

Are there any best practices in ui on how to deal with this sort of conditional execution? Ideally, we'd run the test only once; where would test go?

On Aug 1, 2011, at 4:48 PM, scottgonzalez wrote:

Can we create an element, set the offset to have fractions and then use jQuery to get the offset? Does that give us the information we need?

Reply to this email directly or view it on GitHub:
#190 (comment)

[scottgonzalez]

We'd normally put them in $.ui.support, but that would probably be a bit strange considering it's testing the behavior of jQuery itself. You can just store the result in a variable in the main closure of the position plugin and then do check against that. You can see how we do similar test here. Note that we'll only want to do this in 1-8-stable, not in master, since 1.9 will only support 1.6+. Because of that, we'll want a second pull request. We'll keep this one for master and then have a new one for 1-8-stable.

[kborchers]

While working on better collision detection in position, I made these same changes to remove Math.round from position. The team asked that I take your unit tests and merge them into my pull request, which I have done.

[inukshuk]

I've added a separate request here as you suggested. You'll find a jsfiddle link there, to quickly see the test in action using different jquery versions.