User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.118 Safari/537.36 Steps to reproduce: element.getBoundingClientRect() is frequently used to determine the position of an element on the page. Taking into account the current scroll position it should return a consistent offset position for every element. I noticed the returned values are inconsistent though and I created a simplified test scenario: http://jsfiddle.net/janpaepke/okeg4a7n/ In this scenario I return the current document offset of an element and log it to the console. Actual results: When scrolling up and down, while looking at the console, the values aren't constant. Tested and confirmed also in Firefox Nightly 40.0a1 (2015-04-06) Expected results: The value should be consistant, regardless of scroll position, as the position of the element doesn't change. Safari and Chrome both handle this as expected. Safari doesn't seem to support Sub-Pixel, which might be the reason it works there. Chrome does, though, and it works fine there as well.

The reason for this is that getBoundingClientRect gives you subpixel precision, but window.pageYOffset rounds. If window.pageYOffset were subpixel-precise as well, the value would be consistent.

If that's true, why does the bug not occur on Chrome, where getBoundingClientRect is also sub-pixel precise and window.pageYOffset seems to be rounded? And what would you suggest to use as a reliable form of document related position calculation?

I don't know what Chrome does. Gecko adjusts the page scroll position by a fractional amount once you hit the bottom of the page (if the scrollable area has a fractional height). Chrome probably doesn't do that. As for the reliable form, maybe you can compare with the getBoundingClientRect for a different element? I'd need to do some more testing first.

I tested on IE11. getBoundingClientRect is subpixel precision on IE11 like Firefox.

(In reply to Alice0775 White from comment #4) > I tested on IE11. getBoundingClientRect is subpixel precision on IE11 like > Firefox. I think that's a good thing. getBoundingClientRect SHOULD be sub-pixel precise. But still the returned value should be independent of scroll position or rounding issues in scroll position, don't you agree? What I think may be happening is that Firefox also supports sub-pixels in scrolling, but doesn't return them. So if the total page height is not a full pixel value the scroll position will increment by full px until the bottom of the page is reached. then, because there is still say 0.4 px more to scroll, it will scroll it, causing the real scroll position to be 0.4 px more than what is returned using window.pageYOffset. These 0.4 px will however be substracted from the value returned by getBoundingClientRect, as it is related to viewport and the viewport is 0.4 px further down. But as these 0.4 px are not reflected in window.pageYOffset because it is rounded (as Markus says) the calculated position for the element changes. One solution would be to return the real, unrounded scroll position. But my guess is there's a convention somewhere that says it needs to be full px. So how about getBoundingClientRect returns the position of the element as if the viewport was where the returned scroll position suggests it is?

* These 0.4 px however be ADDED TO the value (sorry)

Hi, This is a bit too technical for me to start debugging, but this looks like related to Bug 1014738. Maybe it would be best if someone with more experience on the matter would take a look over this.

The right solution here is to change window.pageYOffset to return non-integer values, if that's web-compatible. Looks like Blink has in fact done that, so maybe it is. I thought we had an existing bug on it, but I don't see one. I'm not sure what set of props we need to switch over all at once, but seems like at least scrollX/Y and pageX/YOffset on Window should change all together. And maybe scrollTop/Left on elements, though note that scrollWidth/Height on elements are still ints in Blink.

Comment on attachment 8843055 [details] Bug 1151421 Part 1: Change Window scroll and offset properties to be doubles instead of ints. https://reviewboard.mozilla.org/r/116802/#review118810 We need some test for this, hopefully wpt test. Since scrollTo etc already use double, hopefully writing a test should be quite easy. At least some test that if subpixel scrolling is happening, right values are then reported in the attributes. r+, assuming a test is written.

(In reply to Markus Stange [:mstange] from comment #3) > I don't know what Chrome does. Gecko adjusts the page scroll position by a > fractional amount once you hit the bottom of the page (if the scrollable > area has a fractional height). Markus, can you direct me to the code that does this? I'm trying to write a test for the patch, and I can only trigger the test condition with manual scrolling. Instead of rigging a test to simulate a mouse input just for this effect, I'd like to inspect the code path and see if there is an API I can use to set the offset for testing (scrollTo rounds to layer pixels).

We changed this in bug 1012752 so that it no longer happens. We now try to have the scroll position always be a multiple of the device pixel size.

Comment on attachment 8846923 [details] Bug 1151421 Part 2: Add test of fractional scroll position properties. https://reviewboard.mozilla.org/r/119946/#review121976 If there no way to check that we actually get fractional scroll position occasionally? But this at least verifies sane (content.getBoundingClientRect().top + cw.pageYOffset); handling

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7ce372883419 Part 1: Change Window scroll and offset properties to be doubles instead of ints. r=smaug https://hg.mozilla.org/integration/autoland/rev/a84ec98ccaab Part 2: Add test of fractional scroll position properties. r=smaug

Comment on attachment 8846923 [details] Bug 1151421 Part 2: Add test of fractional scroll position properties. https://reviewboard.mozilla.org/r/119946/#review121976 There's currently no setter that can set a fractional pixel -- it gets rounded off to "Layer" pixels pretty deep in the call stack. The original testcase of forced scrolling to the bottom was the only way I could find to trigger a fractional pageYOffset. I'm sorry to say that I missed this review comment before landing the patches. I will open a new bug to modify the test to check that pageYOffset has a fractional component after the scroll.

Backout by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/486692b1d179 Backed out changeset a84ec98ccaab https://hg.mozilla.org/integration/autoland/rev/0bf31f0819dc Backed out changeset 7ce372883419 for failures in Android mochitests, e.g. test_bug1151421.html. r=backout

Backed out for failures in Android mochitests, e.g. test_bug1151421.html: https://hg.mozilla.org/integration/autoland/rev/0bf31f0819dc20a0736803b814d4f88f95609031 https://hg.mozilla.org/integration/autoland/rev/486692b1d1792d304deeb10ca2d657cba790aa9c Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a84ec98ccaabd854a38d9d33b632b39f500ef7e8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=83737591&repo=autoland [task 2017-03-14T16:36:39.708242Z] 16:36:39 INFO - 71 INFO TEST-START | docshell/test/test_bug1151421.html [task 2017-03-14T16:36:39.711096Z] 16:36:39 INFO - Buffered messages logged at 16:36:34 [task 2017-03-14T16:36:39.716943Z] 16:36:39 INFO - 72 INFO TEST-PASS | docshell/test/test_bug1151421.html | top plus offset should remain invariant across scrolling. [task 2017-03-14T16:36:39.717005Z] 16:36:39 INFO - 73 INFO TEST-PASS | docshell/test/test_bug1151421.html | top plus offset should remain invariant across scrolling. [task 2017-03-14T16:36:39.717055Z] 16:36:39 INFO - Buffered messages finished [task 2017-03-14T16:36:39.717141Z] 16:36:39 INFO - 74 INFO TEST-UNEXPECTED-FAIL | docshell/test/test_bug1151421.html | top plus offset should remain invariant across scrolling. - got 87.99999594688416, expected 88 [task 2017-03-14T16:36:39.717198Z] 16:36:39 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:310:5 [task 2017-03-14T16:36:39.717262Z] 16:36:39 INFO - childLoad2/<@docshell/test/test_bug1151421.html:43:5 [task 2017-03-14T16:36:39.717323Z] 16:36:39 INFO - childLoad2@docshell/test/test_bug1151421.html:41:3 More failures in https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2d9fb6e6b9ca1771622f6df413ccec2539d2708d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Comment on attachment 8847224 [details] Bug 1151421 Part 3: Update tests of pageYOffset/scrollY to round off when checking expected results. https://reviewboard.mozilla.org/r/120232/#review122944 I think tests comparing to 0 should work without any changes. It is rather important to not break that case. Also, I didn't see test failures comparing about those 0 case. Or did I miss something? ::: docshell/test/navigation/file_scrollRestoration.html:29 (Diff revision 4) > window.location.reload(false); > break; > } > case 2: { > opener.is(event.persisted, false, "Shouldn't have persisted session history entry."); > - opener.isnot(window.scrollY, 0, "Should have restored scrolling."); > + opener.isnot(Math.round(window.scrollY), 0, "Should have restored scrolling."); Why you need to change any of these window.scrollY != 0 tests? That is rather surprising. Which case is failing on Android? And even scrollY = 0 case... if expect that we haven't scrolled at all, how come we get non 0?

Comment on attachment 8847224 [details] Bug 1151421 Part 3: Update tests of pageYOffset/scrollY to round off when checking expected results. https://reviewboard.mozilla.org/r/120232/#review123012 ::: docshell/test/navigation/file_scrollRestoration.html:29 (Diff revision 4) > window.location.reload(false); > break; > } > case 2: { > opener.is(event.persisted, false, "Shouldn't have persisted session history entry."); > - opener.isnot(window.scrollY, 0, "Should have restored scrolling."); > + opener.isnot(Math.round(window.scrollY), 0, "Should have restored scrolling."); I changed ALL the tests of scroll and pageOffset to round off, for consistency. It's possible these initial condition tests don't fail. I'll scale back to only round on non-zero tests and we'll see what the try server says.

Comment on attachment 8847224 [details] Bug 1151421 Part 3: Update tests of pageYOffset/scrollY to round off when checking expected results. https://reviewboard.mozilla.org/r/120232/#review123012 > I changed ALL the tests of scroll and pageOffset to round off, for consistency. It's possible these initial condition tests don't fail. I'll scale back to only round on non-zero tests and we'll see what the try server says. With most recent patch, I changed all of the is 0 tests to remove the rounding. For isnot 0, it seemed clear that rounding was appropriate, otherwise 0.000111 != 0, clearly against the intent of the test.

Comment on attachment 8847224 [details] Bug 1151421 Part 3: Update tests of pageYOffset/scrollY to round off when checking expected results. https://reviewboard.mozilla.org/r/120232/#review123116

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/77abcda78b97 Part 1: Change Window scroll and offset properties to be doubles instead of ints. r=smaug https://hg.mozilla.org/integration/autoland/rev/8c993a5d99b8 Part 2: Add test of fractional scroll position properties. r=smaug https://hg.mozilla.org/integration/autoland/rev/a2ee4de4944c Part 3: Update tests of pageYOffset/scrollY to round off when checking expected results. r=smaug

https://hg.mozilla.org/mozilla-central/rev/77abcda78b97 https://hg.mozilla.org/mozilla-central/rev/8c993a5d99b8 https://hg.mozilla.org/mozilla-central/rev/a2ee4de4944c

Landed a patch that resolved the problems in the backout in comment 20.

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/scrollx-scrolly-pagexoffset-pageyoffset-now-return-double-instead-of-integer/

I intended to do the same change in bug 1217330 but ceased because of bug 1012752. Given bug 1012752 is fixed, we probably should also try to fix the rest part of bug 1217330 which includes Window.scroll{Max,Min}{X,Y} and Element.scroll{Left,Top}{,Max,Min}. Also I guess this kind of change may be worth an "Intend to ship"?

Documentation updated: https://developer.mozilla.org/en-US/docs/Web/API/Window/scrollX https://developer.mozilla.org/en-US/docs/Web/API/Window/scrollY https://developer.mozilla.org/en-US/Firefox/Releases/55