From 4fd939e8c9dd97c45e7fcd1314f3ab482a2cf23d Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Thu, 10 Aug 2017 18:01:49 +0200 Subject: JS - make window.pageYOffset/pageXOffset/scrollX/scrollY double --- docshell/test/file_bug1151421.html | 19 +++++++ docshell/test/mochitest.ini | 2 + .../test/navigation/file_scrollRestoration.html | 14 ++--- docshell/test/test_bug1151421.html | 61 ++++++++++++++++++++++ docshell/test/test_bug1186774.html | 2 +- docshell/test/test_bug590573.html | 16 +++--- docshell/test/test_bug653741.html | 4 +- docshell/test/test_bug662170.html | 2 +- dom/base/nsGlobalWindow.cpp | 12 ++--- dom/base/nsGlobalWindow.h | 14 ++--- dom/base/test/test_viewport_scroll.html | 4 +- .../mochitest/browserElement_ScrollEvent.js | 4 +- .../general/test_domWindowUtils_scrollXY.html | 12 ++--- dom/webidl/Window.webidl | 12 ++--- layout/forms/test/test_bug562447.html | 6 +-- layout/forms/test/test_bug564115.html | 4 +- 16 files changed, 133 insertions(+), 55 deletions(-) create mode 100644 docshell/test/file_bug1151421.html create mode 100644 docshell/test/test_bug1151421.html diff --git a/docshell/test/file_bug1151421.html b/docshell/test/file_bug1151421.html new file mode 100644 index 0000000000..7bb8c8f363 --- /dev/null +++ b/docshell/test/file_bug1151421.html @@ -0,0 +1,19 @@ + + + + + + +
+
content
+
+ + + diff --git a/docshell/test/mochitest.ini b/docshell/test/mochitest.ini index 725486b771..7b27908fb1 100644 --- a/docshell/test/mochitest.ini +++ b/docshell/test/mochitest.ini @@ -32,6 +32,7 @@ support-files = file_bug680257.html file_bug703855.html file_bug728939.html + file_bug1151421.html file_pushState_after_document_open.html historyframes.html @@ -85,6 +86,7 @@ support-files = file_bug668513.html [test_bug797909.html] [test_bug1045096.html] [test_bug1121701.html] +[test_bug1151421.html] [test_bug1186774.html] [test_forceinheritprincipal_overrule_owner.html] [test_framedhistoryframes.html] diff --git a/docshell/test/navigation/file_scrollRestoration.html b/docshell/test/navigation/file_scrollRestoration.html index 5450c27246..92e43d7fbb 100644 --- a/docshell/test/navigation/file_scrollRestoration.html +++ b/docshell/test/navigation/file_scrollRestoration.html @@ -26,7 +26,7 @@ } 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."); opener.is(history.scrollRestoration, "auto", "Should have the same scrollRestoration as before reload."); history.scrollRestoration = "manual"; window.onunload = function() {} // Disable bfcache. @@ -45,7 +45,7 @@ } case 4: { opener.is(event.persisted, true, "Should have persisted session history entry."); - opener.isnot(window.scrollY, 0, "Should have kept the old scroll position."); + opener.isnot(Math.round(window.scrollY), 0, "Should have kept the old scroll position."); opener.is(history.scrollRestoration, "manual", "Should have the same scrollRestoration as before reload."); window.scrollTo(0, 0); window.location.hash = "hash"; @@ -53,7 +53,7 @@ break; } case 5: { - opener.isnot(window.scrollY, 0, "Should have scrolled to #hash."); + opener.isnot(Math.round(window.scrollY), 0, "Should have scrolled to #hash."); opener.is(history.scrollRestoration, "manual", "Should have the same scrollRestoration mode as before fragment navigation."); window.onunload = function() {} // Disable bfcache. opener.setTimeout("is(testWindow.history.scrollRestoration, 'auto'); testWindow.history.back();", 250); @@ -70,7 +70,7 @@ history.pushState({ state: "state2" }, "state2"); window.scrollTo(0, 0); history.back(); - opener.isnot(window.scrollY, 0, "Should have scrolled back to the state1's position"); + opener.isnot(Math.round(window.scrollY), 0, "Should have scrolled back to the state1's position"); opener.is(history.state.state, "state1", "Unexpected state."); history.scrollRestoration = "manual"; @@ -79,17 +79,17 @@ history.pushState({ state: "state4" }, "state4"); window.scrollTo(0, 0); history.back(); - opener.is(window.scrollY, 0, "Shouldn't have scrolled back to the state3's position"); + opener.is(Math.round(window.scrollY), 0, "Shouldn't have scrolled back to the state3's position"); opener.is(history.state.state, "state3", "Unexpected state."); history.pushState({ state: "state5" }, "state5"); history.scrollRestoration = "auto"; document.getElementById("bottom").scrollIntoView(); - opener.isnot(window.scrollY, 0, "Should have scrolled to 'bottom'."); + opener.isnot(Math.round(window.scrollY), 0, "Should have scrolled to 'bottom'."); history.back(); window.scrollTo(0, 0); history.forward(); - opener.isnot(window.scrollY, 0, "Should have scrolled back to the state5's position"); + opener.isnot(Math.round(window.scrollY), 0, "Should have scrolled back to the state5's position"); var ifr = document.createElement("iframe"); ifr.src = "data:text/html,"; diff --git a/docshell/test/test_bug1151421.html b/docshell/test/test_bug1151421.html new file mode 100644 index 0000000000..76e34d502d --- /dev/null +++ b/docshell/test/test_bug1151421.html @@ -0,0 +1,61 @@ + + + + + Test for Bug 1151421 + + + + +Mozilla Bug 1151421 + + + + +
+ + + + diff --git a/docshell/test/test_bug1186774.html b/docshell/test/test_bug1186774.html index 52ef5f62cb..623e7996b7 100644 --- a/docshell/test/test_bug1186774.html +++ b/docshell/test/test_bug1186774.html @@ -28,7 +28,7 @@ function runTest() { } child.onpopstate = function() { - is(child.scrollY, 6000, "Shouldn't have scrolled before popstate"); + is(Math.round(child.scrollY), 6000, "Shouldn't have scrolled before popstate"); child.close(); SimpleTest.finish(); } diff --git a/docshell/test/test_bug590573.html b/docshell/test/test_bug590573.html index aa6d3bd79f..e218140ea6 100644 --- a/docshell/test/test_bug590573.html +++ b/docshell/test/test_bug590573.html @@ -147,21 +147,21 @@ function* testBody() popup.scroll(0, 100); popup.history.pushState('', '', '?pushed'); - is(popup.scrollY, 100, "test 2"); + is(Math.round(popup.scrollY), 100, "test 2"); popup.scroll(0, 200); // set state-2's position to 200 popup.history.back(); - is(popup.scrollY, 100, "test 3"); + is(Math.round(popup.scrollY), 100, "test 3"); popup.scroll(0, 150); // set original page's position to 150 popup.history.forward(); - is(popup.scrollY, 200, "test 4"); + is(Math.round(popup.scrollY), 200, "test 4"); popup.history.back(); - is(popup.scrollY, 150, "test 5"); + is(Math.round(popup.scrollY), 150, "test 5"); popup.history.forward(); - is(popup.scrollY, 200, "test 6"); + is(Math.round(popup.scrollY), 200, "test 6"); // At this point, the history looks like: // PATH POSITION @@ -202,13 +202,13 @@ function* testBody() is(popup.location.search, "?pushed"); ok(popup.document.getElementById('div1'), 'page should have div1.'); - is(popup.scrollY, 200, "test 8"); + is(Math.round(popup.scrollY), 200, "test 8"); popup.history.back(); - is(popup.scrollY, 150, "test 9"); + is(Math.round(popup.scrollY), 150, "test 9"); popup.history.forward(); - is(popup.scrollY, 200, "test 10"); + is(Math.round(popup.scrollY), 200, "test 10"); // Spin one last time... setTimeout(pageLoad, 0); diff --git a/docshell/test/test_bug653741.html b/docshell/test/test_bug653741.html index f4d4587b8c..a1faf5e2dd 100644 --- a/docshell/test/test_bug653741.html +++ b/docshell/test/test_bug653741.html @@ -27,7 +27,7 @@ function childLoad2() { // Save the Y offset. For sanity's sake, make sure it's not 0, because we // should be at the bottom of the page! - let origYOffset = cw.pageYOffset; + let origYOffset = Math.round(cw.pageYOffset); ok(origYOffset != 0, 'Original Y offset is not 0.'); // Scroll the iframe to the top, then navigate to #bottom again. @@ -37,7 +37,7 @@ function childLoad2() { // bottom again. cw.location = cw.location + ''; - is(cw.pageYOffset, origYOffset, 'Correct offset after reloading page.'); + is(Math.round(cw.pageYOffset), origYOffset, 'Correct offset after reloading page.'); SimpleTest.finish(); } diff --git a/docshell/test/test_bug662170.html b/docshell/test/test_bug662170.html index 514bb55b16..0e626fed48 100644 --- a/docshell/test/test_bug662170.html +++ b/docshell/test/test_bug662170.html @@ -32,7 +32,7 @@ function childLoad2() { cw.scrollTo(0, 300); // Did we actually scroll somewhere? - isnot(cw.pageYOffset, 0, 'Y offset should be non-zero after scrolling.'); + isnot(Math.round(cw.pageYOffset), 0, 'Y offset should be non-zero after scrolling.'); // Now load file_bug662170.html#, which should take us to the top of the // page. diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 8ff4b84ce3..f784031f6a 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -6187,7 +6187,7 @@ nsGlobalWindow::GetScrollMaxY(ErrorResult& aError) FORWARD_TO_OUTER_OR_THROW(GetScrollBoundaryOuter, (eSideBottom), aError, 0); } -CSSIntPoint +CSSPoint nsGlobalWindow::GetScrollXY(bool aDoFlush) { MOZ_ASSERT(IsOuterWindow()); @@ -6211,30 +6211,30 @@ nsGlobalWindow::GetScrollXY(bool aDoFlush) return GetScrollXY(true); } - return sf->GetScrollPositionCSSPixels(); + return CSSPoint::FromAppUnits(scrollPos); } -int32_t +double nsGlobalWindow::GetScrollXOuter() { MOZ_RELEASE_ASSERT(IsOuterWindow()); return GetScrollXY(false).x; } -int32_t +double nsGlobalWindow::GetScrollX(ErrorResult& aError) { FORWARD_TO_OUTER_OR_THROW(GetScrollXOuter, (), aError, 0); } -int32_t +double nsGlobalWindow::GetScrollYOuter() { MOZ_RELEASE_ASSERT(IsOuterWindow()); return GetScrollXY(false).y; } -int32_t +double nsGlobalWindow::GetScrollY(ErrorResult& aError) { FORWARD_TO_OUTER_OR_THROW(GetScrollYOuter, (), aError, 0); diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index eab91c2e4f..dbceeab742 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -1050,15 +1050,15 @@ public: void SetInnerHeight(JSContext* aCx, JS::Handle aValue, mozilla::dom::CallerType aCallerType, mozilla::ErrorResult& aError); - int32_t GetScrollXOuter(); - int32_t GetScrollX(mozilla::ErrorResult& aError); - int32_t GetPageXOffset(mozilla::ErrorResult& aError) + double GetScrollXOuter(); + double GetScrollX(mozilla::ErrorResult& aError); + double GetPageXOffset(mozilla::ErrorResult& aError) { return GetScrollX(aError); } - int32_t GetScrollYOuter(); - int32_t GetScrollY(mozilla::ErrorResult& aError); - int32_t GetPageYOffset(mozilla::ErrorResult& aError) + double GetScrollYOuter(); + double GetScrollY(mozilla::ErrorResult& aError); + double GetPageYOffset(mozilla::ErrorResult& aError) { return GetScrollY(aError); } @@ -1579,7 +1579,7 @@ public: // If aDoFlush is true, we'll flush our own layout; otherwise we'll try to // just flush our parent and only flush ourselves if we think we need to. // Outer windows only. - mozilla::CSSIntPoint GetScrollXY(bool aDoFlush); + mozilla::CSSPoint GetScrollXY(bool aDoFlush); int32_t GetScrollBoundaryOuter(mozilla::Side aSide); diff --git a/dom/base/test/test_viewport_scroll.html b/dom/base/test/test_viewport_scroll.html index 9b812360b2..7db02b7815 100644 --- a/dom/base/test/test_viewport_scroll.html +++ b/dom/base/test/test_viewport_scroll.html @@ -28,10 +28,10 @@ function subtest(winProp, elemProp, win, correctElement, elemToSet, otherElem1, win.scrollTo(50, 50); elemToSet[elemProp] = 100; if (elemToSet == correctElement) { - is(win[winProp], 100, "Setting " + elemToSet.name + "." + elemProp + " should scroll"); + is(Math.round(win[winProp]), 100, "Setting " + elemToSet.name + "." + elemProp + " should scroll"); is(elemToSet[elemProp], 100, "Reading back " + elemToSet.name + "." + elemProp + " after scrolling"); } else { - is(win[winProp], 50, "Setting " + elemToSet.name + "." + elemProp + " should not scroll"); + is(Math.round(win[winProp]), 50, "Setting " + elemToSet.name + "." + elemProp + " should not scroll"); is(elemToSet[elemProp], 0, "Reading back " + elemToSet.name + "." + elemProp + " after not scrolling"); } if (otherElem1 == correctElement) { diff --git a/dom/browser-element/mochitest/browserElement_ScrollEvent.js b/dom/browser-element/mochitest/browserElement_ScrollEvent.js index 5c4b4dcf97..06dc91b862 100644 --- a/dom/browser-element/mochitest/browserElement_ScrollEvent.js +++ b/dom/browser-element/mochitest/browserElement_ScrollEvent.js @@ -16,8 +16,8 @@ function runTest() { iframe.addEventListener("mozbrowserscroll", function(e) { ok(true, "got mozbrowserscroll event."); ok(e.detail, "event.detail is not null."); - ok(e.detail.top === 4000, "top position is correct."); - ok(e.detail.left === 4000, "left position is correct."); + ok(Math.round(e.detail.top) == 4000, "top position is correct."); + ok(Math.round(e.detail.left) == 4000, "left position is correct."); SimpleTest.finish(); }); diff --git a/dom/tests/mochitest/general/test_domWindowUtils_scrollXY.html b/dom/tests/mochitest/general/test_domWindowUtils_scrollXY.html index c6ee89ee35..cf27e5d87f 100644 --- a/dom/tests/mochitest/general/test_domWindowUtils_scrollXY.html +++ b/dom/tests/mochitest/general/test_domWindowUtils_scrollXY.html @@ -31,13 +31,13 @@ function checkGetScrollXYState(flush, vals, testName) { let scrollX = {}, scrollY = {}; domWindowUtils.getScrollXY(flush, scrollX, scrollY); - is(scrollX.value, vals[0], "getScrollXY x for test: " + testName); - is(scrollY.value, vals[1], "getScrollXY y for test: " + testName); + is(Math.round(scrollX.value), vals[0], "getScrollXY x for test: " + testName); + is(Math.round(scrollY.value), vals[1], "getScrollXY y for test: " + testName); } function checkWindowScrollState(vals, testName) { - is(cwindow.scrollX, vals[0], "scrollX for test: " + testName); - is(cwindow.scrollY, vals[1], "scrollY for test: " + testName); + is(Math.round(cwindow.scrollX), vals[0], "scrollX for test: " + testName); + is(Math.round(cwindow.scrollY), vals[1], "scrollY for test: " + testName); } // Check initial state (0, 0) @@ -67,8 +67,8 @@ let scrollX = {}, scrollY = {}; domWindowUtils.getScrollXY(false, scrollX, scrollY); - is(scrollX.value, 0, "scrollX is zero for display:none iframe"); - is(scrollY.value, 0, "scrollY is zero for display:none iframe"); + is(Math.round(scrollX.value), 0, "scrollX is zero for display:none iframe"); + is(Math.round(scrollY.value), 0, "scrollY is zero for display:none iframe"); } SimpleTest.waitForExplicitFinish(); diff --git a/dom/webidl/Window.webidl b/dom/webidl/Window.webidl index 055a274cc1..36b1f0313b 100644 --- a/dom/webidl/Window.webidl +++ b/dom/webidl/Window.webidl @@ -182,14 +182,10 @@ partial interface Window { [ChromeOnly] void mozScrollSnap(); // The four properties below are double per spec at the moment, but whether // that will continue is unclear. - //[Replaceable, Throws] readonly attribute double scrollX; - //[Replaceable, Throws] readonly attribute double pageXOffset; - //[Replaceable, Throws] readonly attribute double scrollY; - //[Replaceable, Throws] readonly attribute double pageYOffset; - [Replaceable, Throws] readonly attribute long scrollX; - [Replaceable, Throws] readonly attribute long pageXOffset; - [Replaceable, Throws] readonly attribute long scrollY; - [Replaceable, Throws] readonly attribute long pageYOffset; + [Replaceable, Throws] readonly attribute double scrollX; + [Throws] readonly attribute double pageXOffset; + [Replaceable, Throws] readonly attribute double scrollY; + [Throws] readonly attribute double pageYOffset; // client // These are writable because we allow chrome to write them. And they need diff --git a/layout/forms/test/test_bug562447.html b/layout/forms/test/test_bug562447.html index 53f84428e5..99f6918711 100644 --- a/layout/forms/test/test_bug562447.html +++ b/layout/forms/test/test_bug562447.html @@ -23,7 +23,7 @@ addLoadEvent(function() { setTimeout(function() { // Make sure that we're scrolled by 5000px - is(window.pageYOffset, 5000, "Make sure we're scrolled correctly"); + is(Math.round(window.pageYOffset), 5000, "Make sure we're scrolled correctly"); // Scroll back up, and mess with the input box along the way var input = document.getElementById("WhyDoYouFocusMe"); @@ -38,14 +38,14 @@ addLoadEvent(function() { window.scrollTo(0, 5000); setTimeout(function() { - is(window.pageYOffset, 5000, "Sanity check"); + is(Math.round(window.pageYOffset), 5000, "Sanity check"); window.scrollTo(0, 0); input.focus(); input.blur(); setTimeout(function() { - isnot(window.pageYOffset, 0, "This time we shouldn't be scrolled up"); + isnot(Math.round(window.pageYOffset), 0, "This time we shouldn't be scrolled up"); SimpleTest.finish(); }, 0); diff --git a/layout/forms/test/test_bug564115.html b/layout/forms/test/test_bug564115.html index 5723b55d5a..ffd4222cae 100644 --- a/layout/forms/test/test_bug564115.html +++ b/layout/forms/test/test_bug564115.html @@ -30,12 +30,12 @@ addLoadEvent(function() { win.scrollTo(0, 5000); setTimeout(function() { - is(win.pageYOffset, 5000, "Page should be scrolled correctly"); + is(Math.round(win.pageYOffset), 5000, "Page should be scrolled correctly"); // Refocus the window SimpleTest.waitForFocus(function() { SimpleTest.waitForFocus(function() { - is(win.pageYOffset, 5000, + is(Math.round(win.pageYOffset), 5000, "The page's scroll offset should not have been changed"); win.close(); -- cgit v1.2.3