third_party/WebKit/LayoutTests/fast/scroll-behavior/visual-viewport-scroll-no-onscroll-event.html - Issue 1416283006: Make window.scroll properties relative to the layout viewport by default.

Side by Side Diff: third_party/WebKit/LayoutTests/fast/scroll-behavior/visual-viewport-scroll-no-onscroll-event.html

Issue 1416283006: Make window.scroll properties relative to the layout viewport by default. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/fast/repaint/fixed-right-bottom-in-page-scale.html ('K') | « third_party/WebKit/LayoutTests/fast/repaint/fixed-right-bottom-in-page-scale.html ('k') | third_party/WebKit/LayoutTests/fast/scroll-behavior/visual-viewport-scroll-no-onscroll-event-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <script src="../../resources/js-test.js"></script>

	3 <style>

	4 #target {

	5 width: 2000px;

	6 height: 1500px;

	7 }

	8 </style>

	9 <script language="JavaScript" type="text/javascript">
	bokan 2015/11/05 14:43:52 Just <script> is fine. Just <script> is fine. ymalik 2015/11/05 15:57:01 Done. Thanks. Show quoted text On 2015/11/05 14:43:52, bokan wrote: > Just <script> is fine. Done. Thanks.
	10 var numScrollsReceived = 0;

	11 var numRAFCalls = 0;

	12 if (window.testRunner && window.internals) {

	13 window.jsTestIsAsync = true;

	14 testRunner.dumpAsText();

	15 testRunner.waitUntilDone();

	16 setPrintTestResultsLazily();

	17 }

	18

	19 description("This test ensures that scrolling the visual viewport doesn't\

	20 trigger an onscroll event.");

	21

	22 function runTest() {

	23 if (!window.testRunner \|\| !window.internals) {

	24 testFailed("This test requires test runner and internals")

	25 finishJSTest();

	26 return;

	27 }

	28

	29 window.internals.setPageScaleFactor(2.0);

	30 var target = document.getElementById('target');

	31

	32 document.onscroll = function() {

	33 ++numScrollsReceived;

	34 debug('OnScroll called for scroll #' + numScrollsReceived);

	35 }

	36

	37 var failureSentinel = function() {

	38 if (numRAFCalls == 0) {

	39 // The first scroll should trigger onscroll because it scrolls

	40 // the layout viewport.

	41 eventSender.mouseMoveTo(target.offsetLeft + 5, target.offsetTop + 5);

	42 eventSender.mouseScrollBy(0, -10);
	bokan 2015/11/05 14:43:52 Maybe I'm missing something but how does this work Maybe I'm missing something but how does this work? Shouldn't the first scroll be at the visual viewport since we scroll visual first now? ymalik 2015/11/05 15:57:01 The -10 here is the number of mouse wheel ticks. S Show quoted text On 2015/11/05 14:43:52, bokan wrote: > Maybe I'm missing something but how does this work? Shouldn't the first scroll > be at the visual viewport since we scroll visual first now? The -10 here is the number of mouse wheel ticks. So we're actually scrolling 400 px, which does scroll the layout viewport. Changed to use continuousMouseScroll to make that more clear. bokan 2015/11/05 16:35:48 Got it, that makes more sense, thanks. Show quoted text On 2015/11/05 15:57:01, ymalik1 wrote: > On 2015/11/05 14:43:52, bokan wrote: > > Maybe I'm missing something but how does this work? Shouldn't the first scroll > > be at the visual viewport since we scroll visual first now? > > The -10 here is the number of mouse wheel ticks. So we're actually scrolling 400 > px, which does scroll the layout viewport. Changed to use continuousMouseScroll > to make that more clear. Got it, that makes more sense, thanks.
	43 } else if (numRAFCalls == 1) {

	44 shouldBe('numScrollsReceived', '1');

	45 // The second scroll should not trigger onscroll because it only

	46 // scrolls the visual viewport.

	47 eventSender.mouseMoveTo(target.offsetLeft + 5, target.offsetTop + 5);

	48 eventSender.mouseScrollBy(0, 1);

	49 } else {

	50 shouldBe('numScrollsReceived', '1');

	51 finishJSTest();
	bokan 2015/11/05 14:43:52 Could you add a return here, it seems we're hittin Could you add a return here, it seems we're hitting this check twice. ymalik 2015/11/05 15:57:01 If we were hitting it twice, it would reflect in t Show quoted text On 2015/11/05 14:43:52, bokan wrote: > Could you add a return here, it seems we're hitting this check twice. If we were hitting it twice, it would reflect in the expected results. I think calling finishJSTest on an async test just terminates it before the call to requestAnimationFrame. But yes, it's more correct to return. bokan 2015/11/05 16:35:48 It is reflected in the expected results, there's t Show quoted text On 2015/11/05 15:57:01, ymalik1 wrote: > On 2015/11/05 14:43:52, bokan wrote: > > Could you add a return here, it seems we're hitting this check twice. > > If we were hitting it twice, it would reflect in the expected results. I think > calling finishJSTest on an async test just terminates it before the call to > requestAnimationFrame. But yes, it's more correct to return. It is reflected in the expected results, there's two "numScrollsReceived is 1" lines. Does this not fix it or did you forget to update the expectation file? ymalik 2015/11/05 16:40:26 The first "numScrollsReceived is 1" is from the pr Show quoted text On 2015/11/05 16:35:48, bokan wrote: > On 2015/11/05 15:57:01, ymalik1 wrote: > > On 2015/11/05 14:43:52, bokan wrote: > > > Could you add a return here, it seems we're hitting this check twice. > > > > If we were hitting it twice, it would reflect in the expected results. I think > > calling finishJSTest on an async test just terminates it before the call to > > requestAnimationFrame. But yes, it's more correct to return. > > It is reflected in the expected results, there's two "numScrollsReceived is 1" > lines. Does this not fix it or did you forget to update the expectation file? The first "numScrollsReceived is 1" is from the previous branch "numRAFCalls == 1". That's when numScrollsReceived is first incremented by the call to onscroll from the layout viewport scroll. bokan 2015/11/05 17:02:47 DOH! Brain fart on my part :/ Show quoted text On 2015/11/05 16:40:26, ymalik1 wrote: > On 2015/11/05 16:35:48, bokan wrote: > > On 2015/11/05 15:57:01, ymalik1 wrote: > > > On 2015/11/05 14:43:52, bokan wrote: > > > > Could you add a return here, it seems we're hitting this check twice. > > > > > > If we were hitting it twice, it would reflect in the expected results. I > think > > > calling finishJSTest on an async test just terminates it before the call to > > > requestAnimationFrame. But yes, it's more correct to return. > > > > It is reflected in the expected results, there's two "numScrollsReceived is 1" > > lines. Does this not fix it or did you forget to update the expectation file? > > The first "numScrollsReceived is 1" is from the previous branch "numRAFCalls == > 1". That's when numScrollsReceived is first incremented by the call to onscroll > from the layout viewport scroll. DOH! Brain fart on my part :/
	52 }

	53 ++numRAFCalls;

	54 window.requestAnimationFrame(failureSentinel);

	55 }

	56

	57 window.requestAnimationFrame(failureSentinel);

	58 }

	59

	60 onload = runTest;

	61 </script>

	62 <div id="target"></div>

OLD	NEW