third_party/WebKit/LayoutTests/fast/dom/Window/window-scaled-viewport-properties.html - Issue 1415513002: Remove plumbing for inert-visual-viewport flag

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

Issue 1415513002: Remove plumbing for inert-visual-viewport flag (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years, 2 months 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 | « content/renderer/render_view_impl.cc ('k') | third_party/WebKit/LayoutTests/fast/dom/Window/window-scaled-viewport-properties-expected.txt » ('j') | third_party/WebKit/LayoutTests/fast/dom/window-inner-size-scaling.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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

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

3 <style>	3 <style>

4 body {	4 body {

5 padding: 0px;	5 padding: 0px;

6 margin: 0px;	6 margin: 0px;

	7 /* Overflow hidden so that the size of the scrollbar is not added to

	8 the innerHeight/Width properties. */

	9 overflow: hidden;

7 }	10 }

8	11

9 .spacer {	12 .spacer {

10 position: absolute;	13 position: absolute;

11 left: 0px;	14 left: 0px;

12 top: 0px;	15 top: 0px;

13 margin: 0px;	16 margin: 0px;

14 padding: 0px;	17 padding: 0px;

15 width: 2000px;	18 width: 2000px;

16 height: 1500px;	19 height: 1500px;

17 }	20 }

18 </style>	21 </style>

19 <script language="JavaScript" type="text/javascript">	22 <script language="JavaScript" type="text/javascript">

20 if (window.testRunner && window.internals) {	23 if (window.testRunner && window.internals) {

21 window.internals.setPageScaleFactorLimits(0.5, 4.0);	24 window.internals.setPageScaleFactorLimits(0.5, 4.0);

22 window.jsTestIsAsync = true;	25 window.jsTestIsAsync = true;

23 testRunner.dumpAsText();	26 testRunner.dumpAsText();

24 testRunner.waitUntilDone();	27 testRunner.waitUntilDone();

25 }	28 }

26	29

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

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

29	32

30 debug('===Unscaled===');	33

31 debug('');	34 function testUnscaled () {

32 shouldBe('window.innerWidth', '800');	35 debug('===Unscaled===');

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

	37 shouldBe('window.innerWidth', '1600');
	bokan 2015/10/19 15:01:14 Can you add a comment here (or above at the setPag Can you add a comment here (or above at the setPageScaleFactorLimits call maybe) that the 0.5 minimum scale causes the layout viewport to become twice what we'd normally expect because the layout viewport is based on minimum page scale. ymalik 2015/10/20 21:27:14 Done. Show quoted text On 2015/10/19 15:01:14, bokan wrote: > Can you add a comment here (or above at the setPageScaleFactorLimits call maybe) > that the 0.5 minimum scale causes the layout viewport to become twice what we'd > normally expect because the layout viewport is based on minimum page scale. Done.
	38 shouldBe('window.innerHeight', '1200');

	39 }

34	40

35 function testPinchedIn() {	41 function testPinchedIn() {

36 debug('');	42 debug('');

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

38 debug('');	44 debug('');

39 window.internals.setPageScaleFactor(2.0);	45 window.internals.setPageScaleFactor(2.0);

40 shouldBe('window.innerWidth', '400');	46 shouldBe('window.innerWidth', '1600');

41 shouldBe('window.innerHeight', '300');	47 shouldBe('window.innerHeight', '1200');

42 shouldBe('window.scrollX', '0');	48 shouldBe('window.scrollX', '0');

43 shouldBe('window.scrollY', '0');	49 shouldBe('window.scrollY', '0');

44	50

45 window.scrollBy(10, 20);	51 window.scrollBy(10, 20);

46 shouldBe('window.scrollX', '10');	52 shouldBe('window.scrollX', '10');

47 shouldBe('window.scrollY', '20');	53 shouldBe('window.scrollY', '20');

48 window.scrollBy(1590, 1180);	54 window.scrollBy(1590, 1180);

49 shouldBe('window.scrollX', '1600');	55 shouldBe('window.scrollX', '400');

50 shouldBe('window.scrollY', '1200');	56 shouldBe('window.scrollY', '300');

51 window.scrollBy(-1600, -1200);	57 window.scrollBy(-1600, -1200);

52 shouldBe('window.scrollX', '0');	58 shouldBe('window.scrollX', '0');

53 shouldBe('window.scrollY', '0');	59 shouldBe('window.scrollY', '0');

54 window.scrollTo(1600, 1200);	60 window.scrollTo(1600, 1200);

55 shouldBe('window.scrollX', '1600');	61 shouldBe('window.scrollX', '400');

56 shouldBe('window.scrollY', '1200');	62 shouldBe('window.scrollY', '300');
	bokan 2015/10/19 15:01:14 You can get rid of most of these scrollBy calls si You can get rid of most of these scrollBy calls since we needed them before to indirectly cause scrolling of one or both viewports. One or two scrolls (perhaps one to some (x,y) and one past the edge, to make sure we don't bubble any) should be enough now. Also, now that we have explicit methods on Internals to check the visual viewport offset, check that it doesn't change because of these scrollTo/By calls. Ditto for the below functions as well. ymalik 2015/10/20 21:27:13 Done. Show quoted text On 2015/10/19 15:01:14, bokan wrote: > You can get rid of most of these scrollBy calls since we needed them before to > indirectly cause scrolling of one or both viewports. One or two scrolls (perhaps > one to some (x,y) and one past the edge, to make sure we don't bubble any) > should be enough now. Also, now that we have explicit methods on Internals to > check the visual viewport offset, check that it doesn't change because of these > scrollTo/By calls. Ditto for the below functions as well. Done. ymalik 2015/10/20 23:36:00 To be more clear about the change. I refactored th Show quoted text On 2015/10/20 21:27:13, ymalik1 wrote: > On 2015/10/19 15:01:14, bokan wrote: > > You can get rid of most of these scrollBy calls since we needed them before to > > indirectly cause scrolling of one or both viewports. One or two scrolls > (perhaps > > one to some (x,y) and one past the edge, to make sure we don't bubble any) > > should be enough now. Also, now that we have explicit methods on Internals to > > check the visual viewport offset, check that it doesn't change because of > these > > scrollTo/By calls. Ditto for the below functions as well. > > Done. To be more clear about the change. I refactored the scrolls to testScrolls. testScrolls scrolls to an (x, y) and past the edge.
57 window.scrollTo(0, 0);	63 window.scrollTo(0, 0);

58 shouldBe('window.scrollX', '0');	64 shouldBe('window.scrollX', '0');

59 shouldBe('window.scrollY', '0');	65 shouldBe('window.scrollY', '0');

60 }	66 }

61	67

62 function testMaximallyPinchedOut() {	68 function testMaximallyPinchedOut() {

63 debug('');	69 debug('');

64 debug('===Pinch Out to 0.5X===');	70 debug('===Pinch Out to 0.5X===');

65 debug('');	71 debug('');

66 window.internals.setPageScaleFactor(0.5);	72 window.internals.setPageScaleFactor(0.5);

(...skipping 17 matching lines...) Expand all Loading...
84 window.scrollTo(0, 0);	90 window.scrollTo(0, 0);

85 shouldBe('window.scrollX', '0');	91 shouldBe('window.scrollX', '0');

86 shouldBe('window.scrollY', '0');	92 shouldBe('window.scrollY', '0');

87 }	93 }

88	94

89 function testOnScroll() {	95 function testOnScroll() {

90 debug('');	96 debug('');

91 debug('===Test OnScroll===');	97 debug('===Test OnScroll===');

92 debug('');	98 debug('');

93 window.internals.setPageScaleFactor(1.0);	99 window.internals.setPageScaleFactor(1.0);

94 shouldBe('window.innerWidth', '800');	100 shouldBe('window.innerWidth', '1600');

95 shouldBe('window.innerHeight', '600');	101 shouldBe('window.innerHeight', '1200');

96 shouldBe('window.scrollX', '0');	102 shouldBe('window.scrollX', '0');

97 shouldBe('window.scrollY', '0');	103 shouldBe('window.scrollY', '0');

98	104

99 // First scroll scrolls only the outer viewport.	105 // First scroll should only scroll the visual viewport because the
	bokan 2015/10/19 15:01:14 None of the scrolls should be going to the visual None of the scrolls should be going to the visual viewport anymore, right? Is it just the comment that's out of date? ymalik 2015/10/20 23:36:00 Yes, fixed the comment. Show quoted text On 2015/10/19 15:01:14, bokan wrote: > None of the scrolls should be going to the visual viewport anymore, right? Is it > just the comment that's out of date? Yes, fixed the comment.
	106 // layout and visual viewport are of different sizes (because of the

	107 // call to setPageScaleFactorLimits above).

100 // Second scrolls the outer and the inner.	108 // Second scrolls the outer and the inner.

101 // Third scrolls only the inner.	109 // Third scroll should have no affect because the layout viewport is

	110 // fully scrolled.

102 var scrolls = [100, 400, 100];	111 var scrolls = [100, 400, 100];

103 var numScrollsReceived = 0;	112 var numScrollsReceived = 0;

104 var numRAFCalls = 0;	113 var numRAFCalls = 0;

105	114

106 document.onscroll = function() {	115 document.onscroll = function() {

107 if (numRAFCalls == 0)	116 if (numRAFCalls == 0)

108 return;	117 return;

109	118

110 ++numScrollsReceived;	119 ++numScrollsReceived;

111 debug('PASS OnScroll called for scroll #' + numScrollsReceived);	120 debug('PASS OnScroll called for scroll #' + numScrollsReceived);

112 if (numScrollsReceived < scrolls.length) {	121 if (numScrollsReceived < scrolls.length) {

113 var scrollAmount = scrolls[numScrollsReceived];	122 var scrollAmount = scrolls[numScrollsReceived];

114 window.scrollBy(scrollAmount, 0);	123 window.scrollBy(scrollAmount, 0);
	bokan 2015/10/19 15:01:14 scrollBy won't ever scroll the inner viewport. Thi scrollBy won't ever scroll the inner viewport. This function is testing that scroll events are properly delivered. I don't think we ever explicitly talked about this, but I think the right thing now would be to only send onScrolls for layout viewport scrolling, and not for visual. In that case, we still want to make sure that's the case so you'll probably want to use the event sender to send wheel/touch events that cause scrolling. That'll ensure that onScroll isn't triggered for visual scrolls. ymalik 2015/10/20 23:36:00 Yes, this test doesn't cover the case when we actu Show quoted text On 2015/10/19 15:01:14, bokan wrote: > scrollBy won't ever scroll the inner viewport. This function is testing that > scroll events are properly delivered. I don't think we ever explicitly talked > about this, but I think the right thing now would be to only send onScrolls for > layout viewport scrolling, and not for visual. > > In that case, we still want to make sure that's the case so you'll probably want > to use the event sender to send wheel/touch events that cause scrolling. That'll > ensure that onScroll isn't triggered for visual scrolls. Yes, this test doesn't cover the case when we actually scroll the visual viewport. Added a new test, see visual-viewport-scroll-no-onscroll-event.html below.
115 } else if (numScrollsReceived == scrolls.length) {	124 } else if (numScrollsReceived == scrolls.length) {

116 // Make sure scrollTo that moves only the inner viewport also	125 testFailed("Received visual viewport scroll event");

117 // triggers a scroll event.	126 finishJSTest();

118 window.scrollTo(1200, 0);

119 } else {	127 } else {

120 debug('');	128 debug('');

121 finishJSTest();	129 finishJSTest();

122 }	130 }

123 }	131 }

124	132

125 // Scroll events are fired right before RAF so this is a good place to	133 // Scroll events are fired right before RAF so this is a good place to

126 // make sure event was handled.	134 // make sure event was handled.

127 var failureSentinel = function() {	135 var failureSentinel = function() {

128 if (numRAFCalls == 0) {	136 if (numRAFCalls == 0) {

129 window.scrollBy(scrolls[0], 0);	137 window.scrollBy(scrolls[0], 0);

130 }else if (numRAFCalls > numScrollsReceived) {	138 } else if (numRAFCalls == scrolls.length) {

131 testFailed("Failed to receive scroll event #" + (numScrollsRecei ved+1));	139 testPassed("Did not receive visual viewport scroll event");

	140 debug('');

	141 finishJSTest();

	142 } else if (numRAFCalls > numScrollsReceived) {

	143 testFailed("Did not receive scroll event #" + (numScrollsReceive d+1));

132 finishJSTest();	144 finishJSTest();

133 }	145 }

134 ++numRAFCalls;	146 ++numRAFCalls;

135 window.requestAnimationFrame(failureSentinel);	147 window.requestAnimationFrame(failureSentinel);

136 }	148 }

137	149

138 window.requestAnimationFrame(failureSentinel);	150 window.requestAnimationFrame(failureSentinel);

139 }	151 }

140	152

141 function forceLayout() {	153 function forceLayout() {

142 window.scrollTo(0, 0);	154 window.scrollX;

143 }	155 }

144	156

145 function runTests() {	157 function runTests() {

146 if (window.testRunner && window.internals) {	158 if (window.testRunner && window.internals) {

	159 // TODO(ymalik): The call to setPageScaleFactorLimits should force

	160 // layout. Fix that instead of forcing layout here.

147 forceLayout();	161 forceLayout();

	162 testUnscaled();

148 testPinchedIn();	163 testPinchedIn();

149 testMaximallyPinchedOut();	164 testMaximallyPinchedOut();

150 testOnScroll();	165 testOnScroll();

151 }	166 }

152 }	167 }

153	168

154 onload = runTests;	169 onload = runTests;

155 </script>	170 </script>

156 <div class="spacer"></div>	171 <div class="spacer"></div>

OLD	NEW