LayoutTests/fast/dom/Window/window-scaled-viewport-properties.html - Issue 731413002: Adjust DOMWindow properties to emulate old style pinch semantics

Side by Side Diff: LayoutTests/fast/dom/Window/window-scaled-viewport-properties.html

Issue 731413002: Adjust DOMWindow properties to emulate old style pinch semantics (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Added test Created 6 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

« no previous file with comments | « LayoutTests/TestExpectations ('k') | LayoutTests/fast/dom/Window/window-scaled-viewport-properties-expected.txt » ('j') | Source/core/frame/LocalDOMWindow.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

	2 <html>
	Rick Byers 2014/11/18 20:53:31 nit: omit html, head and body tags: http://www.chr nit: omit html, head and body tags: http://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-h... bokan 2014/11/18 23:21:50 Done. Show quoted text On 2014/11/18 20:53:31, Rick Byers wrote: > nit: omit html, head and body tags: > http://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-h... Done.
	3 <head>

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

	5 <style>

	6 body {

	7 padding: 0px;

	8 margin: 0px;

	9 }

	10

	11 .spacer {

	12 position: absolute;

	13 left: 0px;

	14 top: 0px;

	15 margin: 0px;

	16 padding: 0px;

	17 width: 2000px;

	18 height: 1500px;

	19 }

	20 </style>

	21 </head>

	22 <body>

	23 <script language="JavaScript" type="text/javascript">

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

	25 window.jsTestIsAsync = true;

	26 window.eventSender.setPageScaleFactorLimits(0.25, 4.0);

	27 testRunner.dumpAsText();

	28 testRunner.waitUntilDone();

	29 }

	30

	31 description("This test makes sure the window properties related to the\

	32 viewport remain correct under pinch-to-zoom.");

	33

	34 debug('===Unscaled===');

	35 shouldBe('window.innerWidth', '800');

	36 shouldBe('window.innerHeight', '600');

	37

	38 function testPinchedIn() {

	39 debug('');

	40 debug('===Pinch Zoom in to 2X===');

	41 debug('');

	42 window.eventSender.setPageScaleFactor(2.0, 0, 0);

	43 shouldBe('window.innerWidth', '400');

	44 shouldBe('window.innerHeight', '300');

	45 shouldBe('window.scrollX', '0');

	46 shouldBe('window.scrollY', '0');

	47

	48 window.scrollBy(10, 20);

	49 shouldBe('window.scrollX', '10');

	50 shouldBe('window.scrollY', '20');

	51 window.scrollBy(1590, 1180);

	52 shouldBe('window.scrollX', '1600');

	53 shouldBe('window.scrollY', '1200');

	54 window.scrollBy(-1600, -1200);

	55 shouldBe('window.scrollX', '0');

	56 shouldBe('window.scrollY', '0');

	57 window.scrollTo(1600, 1200);

	58 shouldBe('window.scrollX', '1600');

	59 shouldBe('window.scrollY', '1200');

	60 window.scrollTo(0, 0);

	61 shouldBe('window.scrollX', '0');

	62 shouldBe('window.scrollY', '0');

	63 }

	64

	65 function testPinchedOut() {

	66 debug('');

	67 debug('===Pinch Out to 0.5X===');

	68 debug('');

	69 window.eventSender.setPageScaleFactor(0.5, 0, 0);

	70 shouldBe('window.innerWidth', '1600');

	71 shouldBe('window.innerHeight', '1200');

	72 shouldBe('window.scrollX', '0');

	73 shouldBe('window.scrollY', '0');

	74

	75 window.scrollBy(10, 20);

	76 shouldBe('window.scrollX', '10');

	77 shouldBe('window.scrollY', '20');

	78 window.scrollBy(390, 280);

	79 shouldBe('window.scrollX', '400');

	80 shouldBe('window.scrollY', '300');

	81 window.scrollBy(-400, -300);

	82 shouldBe('window.scrollX', '0');

	83 shouldBe('window.scrollY', '0');

	84 window.scrollTo(400, 300);

	85 shouldBe('window.scrollX', '400');

	86 shouldBe('window.scrollY', '300');

	87 window.scrollTo(0, 0);

	88 shouldBe('window.scrollX', '0');

	89 shouldBe('window.scrollY', '0');

	90 }

	91

	92 function testMaximallyPinchedOut() {

	93 debug('');

	94 debug('===Pinch Out to 0.4X (Minimum Scale)===');

	95 debug('');

	96 window.eventSender.setPageScaleFactor(0.4, 0, 0);

	97 shouldBe('window.innerWidth', '2000');

	98 shouldBe('window.innerHeight', '1500');

	99 shouldBe('window.scrollX', '0');

	100 shouldBe('window.scrollY', '0');

	101

	102 window.scrollBy(10000, 10000);

	103 shouldBe('window.scrollX', '0');

	104 shouldBe('window.scrollY', '0');

	105 window.scrollTo(10000, 10000);

	106 shouldBe('window.scrollX', '0');

	107 shouldBe('window.scrollY', '0');

	108 }

	109

	110 function runTests() {

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

	112 testPinchedIn();

	113 testPinchedOut();

	114 testMaximallyPinchedOut();

	115 testRunner.notifyDone();

	116 }

	117 }

	118

	119 setTimeout(runTests, 100);
	Rick Byers 2014/11/18 20:53:31 where does this 100 come from? You should be able where does this 100 come from? You should be able to start running immediately, no? Otherwise it may be flaky. If you like you can wait until the load event (eg. 'onload = function() { ...'). But please don't add arbitrary timeouts to tests. bokan 2014/11/18 23:21:50 Yah, I was worried about flakiness but there's som Show quoted text On 2014/11/18 20:53:31, Rick Byers wrote: > where does this 100 come from? You should be able to start running immediately, > no? Otherwise it may be flaky. If you like you can wait until the load event > (eg. 'onload = function() { ...'). But please don't add arbitrary timeouts to > tests. Yah, I was worried about flakiness but there's some initialization issues related to onload (crbug.com/343477) so I wanted to run the test after onload. Anyway, after you mentioned this I realized adding scrollTo(0,0) will force a layout which is what I needed so Done. Rick Byers 2014/11/20 19:23:30 Great. Note that there are lots of tests that exp Show quoted text On 2014/11/18 23:21:50, bokan wrote: > On 2014/11/18 20:53:31, Rick Byers wrote: > > where does this 100 come from? You should be able to start running > immediately, > > no? Otherwise it may be flaky. If you like you can wait until the load event > > (eg. 'onload = function() { ...'). But please don't add arbitrary timeouts to > > tests. > > Yah, I was worried about flakiness but there's some initialization issues > related to onload (crbug.com/343477) so I wanted to run the test after onload. > Anyway, after you mentioned this I realized adding scrollTo(0,0) will force a > layout which is what I needed so Done. Great. Note that there are lots of tests that explicitly provoke a layout (eg. by reading a property like offsetTop). It's also fine to use setTimeout(0). But any non-zero value is almost always a test bug...
	120

	121 </script>
	Rick Byers 2014/11/18 20:53:31 you also fixed a bug with firing missing 'scroll' you also fixed a bug with firing missing 'scroll' events, right? Please test that case too. bokan 2014/11/18 23:21:50 Right, thanks, added. The only issue is that scrol Show quoted text On 2014/11/18 20:53:31, Rick Byers wrote: > you also fixed a bug with firing missing 'scroll' events, right? Please test > that case too. Right, thanks, added. The only issue is that scroll events are sent async on new frames (I think) so I added an "(quasi-)arbitrary timeout" since otherwise the test will wait an eternity before failing and wont give output. Is there a better way we handle this case? I think I've seen similar patterns in other layout tests. Rick Byers 2014/11/20 19:23:30 I'm a little worried about this being flaky - eg. Show quoted text On 2014/11/18 23:21:50, bokan wrote: > On 2014/11/18 20:53:31, Rick Byers wrote: > > you also fixed a bug with firing missing 'scroll' events, right? Please test > > that case too. > > Right, thanks, added. The only issue is that scroll events are sent async on new > frames (I think) so I added an "(quasi-)arbitrary timeout" since otherwise the > test will wait an eternity before failing and wont give output. Is there a > better way we handle this case? I think I've seen similar patterns in other > layout tests. I'm a little worried about this being flaky - eg. on Valgrind bots or something. scroll events are fired immediately before RAF every frame. So what about putting your timeout inside of a RAF callback? If a RAF callback occurs without having seen a scroll, then you should be able to fail completely reliably. That should also be simpler than what you have :-). bokan 2014/11/20 20:30:41 That worked perfectly. Thanks for keeping us hones Show quoted text On 2014/11/20 19:23:30, Rick Byers wrote: > On 2014/11/18 23:21:50, bokan wrote: > > On 2014/11/18 20:53:31, Rick Byers wrote: > > > you also fixed a bug with firing missing 'scroll' events, right? Please > test > > > that case too. > > > > Right, thanks, added. The only issue is that scroll events are sent async on > new > > frames (I think) so I added an "(quasi-)arbitrary timeout" since otherwise the > > test will wait an eternity before failing and wont give output. Is there a > > better way we handle this case? I think I've seen similar patterns in other > > layout tests. > > I'm a little worried about this being flaky - eg. on Valgrind bots or something. > scroll events are fired immediately before RAF every frame. So what about > putting your timeout inside of a RAF callback? If a RAF callback occurs without > having seen a scroll, then you should be able to fail completely reliably. That > should also be simpler than what you have :-). That worked perfectly. Thanks for keeping us honest!
	122 <div class="spacer"></div>

	123 </body>

	124 </html>

OLD	NEW