third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollIntoView.html - Issue 2936283003: Update CSSOM SmoothScroll Web Platform Test

Side by Side Diff: third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollIntoView.html

Issue 2936283003: Update CSSOM SmoothScroll Web Platform Test (Closed)

Patch Set: format Created 3 years, 6 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:

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html » ('j') | third_party/WebKit/Source/core/dom/Element.idl » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE HTML>

	2 <script src='/resources/testharness.js'></script>

	3 <script src='/resources/testharnessreport.js'></script>

	4 <title>Check End Position of scrollIntoView</title>

	5 <div id='container' style='height: 2500px; width: 2500px;'>

	6 <div id='content' style='height: 500px; width: 500px;margin-left: 1000px; marg in-right: 1000px; margin-top: 1000px;margin-bottom: 1000px;background-color: red '>

	7 </div>

	8 <div id='shadow'></div>

	9 </div>

	10 <script>

	11

	12 var frames = 0;

	13 var last_changed_frame = 0;

	14 var last_changed_x = 0;

	15 var last_changed_y = 0;

	16 var expected_x = 0;

	17 var expected_y = 0;

	18 var content_height = 500;

	19 var content_width = 500;

	20 var window_height = document.documentElement.clientHeight;

	21 var window_width = document.documentElement.clientWidth;

	22 var content = document.getElementById('content');

	23 add_completion_callback(() => document.getElementById('container').remove());

	24

	25 function animate (test, next_test) {
	foolip 2017/06/15 08:38:04 Optional nits/suggestions for how you could have l Optional nits/suggestions for how you could have less global state and manual serialization of tests here. Unlike async_test, promise_test will run one after the other by default. By also making animate return a promise, I think you could simplify the first test into something like this: function waitForScrollEnd() { return new Promise((resolve, reject) => { // A rAF loop that waits for 20 frames without change and rejects // if it doesn't stop, or maybe just let the test time out. resolve(); }); } promise_test(t => { // assert preconditions or maybe scroll to required start position content.scrollIntoView({behavior: 'smooth', block: 'nearest', inlinePosition: 'nearest'}); return waitForScrollEnd().then(() => { assert_approx_equals(window.scrollX, content.offsetLeft, 1); assert_approx_equals(window.scrollX, content.offsetTop, 1); }); }, "Smooth scrollIntoView should scroll the element to the 'nearest' position"); The other tests would be similar. If you try it and you don't like the look of it, feel free to ignore. sunyunjia 2017/06/15 18:00:26 Thanks! Finally learned how to use promise today! Show quoted text On 2017/06/15 08:38:04, foolip wrote: > Optional nits/suggestions for how you could have less global state and manual > serialization of tests here. Unlike async_test, promise_test will run one after > the other by default. By also making animate return a promise, I think you could > simplify the first test into something like this: > > function waitForScrollEnd() { > return new Promise((resolve, reject) => { > // A rAF loop that waits for 20 frames without change and rejects > // if it doesn't stop, or maybe just let the test time out. > resolve(); > }); > } > > promise_test(t => { > // assert preconditions or maybe scroll to required start position > content.scrollIntoView({behavior: 'smooth', block: 'nearest', inlinePosition: > 'nearest'}); > return waitForScrollEnd().then(() => { > assert_approx_equals(window.scrollX, content.offsetLeft, 1); > assert_approx_equals(window.scrollX, content.offsetTop, 1); > }); > }, "Smooth scrollIntoView should scroll the element to the 'nearest' position"); > > The other tests would be similar. If you try it and you don't like the look of > it, feel free to ignore. Thanks! Finally learned how to use promise today!
	26 if (frames < 500 && frames - last_changed_frame < 20) {
	foolip 2017/06/15 08:38:04 Can you add a comment about what this means, that Can you add a comment about what this means, that we rAF either for 500 frames or until 20 frames with no change have been observed? sunyunjia 2017/06/15 18:00:26 Done. Show quoted text On 2017/06/15 08:38:04, foolip wrote: > Can you add a comment about what this means, that we rAF either for 500 frames > or until 20 frames with no change have been observed? Done.
	27 ++frames;

	28 if (window.scrollX != last_changed_x \|\| window.scrollY != last_changed_y) {

	29 last_changed_x = window.scrollX;

	30 last_changed_y = window.scrollY;

	31 last_changed_frame = frames;

	32 }

	33 requestAnimationFrame(animate.bind(null, test, next_test));

	34 } else {

	35 if (test) {

	36 test.step(function() {

	37 assert_approx_equals(window.scrollX, expected_x, 1);

	38 assert_approx_equals(window.scrollY, expected_y, 1);

	39 test.done();

	40 });

	41 }

	42 if (next_test)

	43 next_test();

	44 }

	45 }

	46

	47 var checkNearest = async_test("Smooth scrollIntoView should scroll the element t o the 'nearest' position");

	48 function test1() {

	49 checkNearest.step(function() {

	50 content.scrollIntoView(

	51 {behavior: 'smooth', block: 'nearest', inlinePosition: 'nearest'});

	52 frames = 0;

	53 last_changed_frame = 0;

	54 last_changed_x = window.scrollX;

	55 last_changed_y = window.scrollY;

	56 expected_x = content.offsetLeft + content_width - window_width;

	57 expected_y = content.offsetTop + content_height - window_height;

	58 animate(checkNearest, test2);

	59 });

	60 }

	61

	62 var checkStart = async_test("Smooth scrollIntoView should scroll the element to the 'start' position");

	63 function test2() {

	64 checkStart.step(function() {

	65 content.scrollIntoView(

	66 {behavior: 'smooth', block: 'start', inlinePosition: 'start'});

	67 frames = 0;

	68 last_changed_frame = 0;

	69 last_changed_x = window.scrollX;

	70 last_changed_y = window.scrollY;

	71 expected_x = content.offsetLeft;

	72 expected_y = content.offsetTop;

	73 animate(checkStart, test3);

	74 });

	75 }

	76

	77 var checkCenter = async_test("Smooth scrollIntoView should scroll the element to the 'center' position");

	78 function test3() {

	79 checkCenter.step(function() {

	80 content.scrollIntoView(

	81 {behavior: 'smooth', block: 'center', inlinePosition: 'center'});

	82 frames = 0;

	83 last_changed_frame = 0;

	84 last_changed_x = window.scrollX;

	85 last_changed_y = window.scrollY;

	86 expected_x = content.offsetLeft + (content_width - window_width) / 2;

	87 expected_y = content.offsetTop + (content_height - window_height) / 2;

	88 animate(checkCenter, test4);

	89 });

	90 }

	91

	92 var checkEnd = async_test("Smooth scrollIntoView should scroll the element to th e 'end' position");

	93 function test4() {

	94 checkEnd.step(function() {

	95 content.scrollIntoView(

	96 {behavior: 'smooth', block: 'end', inlinePosition: 'end'});

	97 frames = 0;

	98 last_changed_frame = 0;

	99 last_changed_x = window.scrollX;

	100 last_changed_y = window.scrollY;

	101 expected_x = content.offsetLeft + content_width - window_width;

	102 expected_y = content.offsetTop + content_height - window_height;

	103 animate(checkEnd, test5);

	104 });

	105 }

	106

	107 var checkEmptyAndNullAndUndefined = async_test("scrollIntoView should behave cor rectly when the arg is empty, null or undefined");

	108 function test5() {

	109 checkEmptyAndNullAndUndefined.step(function() {

	110 content.scrollIntoView();

	111 var x = content.offsetLeft + content_width - window_width;

	112 var y = content.offsetTop;

	113 assert_approx_equals(window.scrollX, x, 1);

	114 assert_approx_equals(window.scrollY, y, 1);

	115

	116 content.scrollIntoView(null);

	117 x = content.offsetLeft + (content_width - window_width) / 2;

	118 y = content.offsetTop + (content_height - window_height) / 2;

	119 assert_approx_equals(window.scrollX, x, 1);

	120 assert_approx_equals(window.scrollY, y, 1);

	121

	122 window.scrollTo(0, 0);

	123 assert_approx_equals(window.scrollX, 0, 1);

	124 assert_approx_equals(window.scrollY, 0, 1);

	125

	126 content.scrollIntoView(undefined);

	127 assert_approx_equals(window.scrollX, x, 1);

	128 assert_approx_equals(window.scrollY, y, 1);

	129

	130 checkEmptyAndNullAndUndefined.done();

	131 test6();

	132 })

	133 }

	134

	135 var scrollShadowDomElementIntoView = async_test("scrollIntoView should behave co rrectly if applied to shadow dom elements");

	136 function test6() {

	137 scrollShadowDomElementIntoView.step(function() {

	138 var shadow = document.getElementById("shadow");

	139 var shadowRoot = shadow.createShadowRoot();

	140 var shadowDiv = document.createElement("div");

	141 shadowDiv.style.height = "200px";

	142 shadowDiv.style.width = "200px";

	143 shadowDiv.style.backgroundColor = "green";

	144 shadowRoot.appendChild(shadowDiv);

	145

	146 shadowDiv.scrollIntoView({block: 'start', inlinePosition: 'start'});

	147 assert_approx_equals(window.scrollX, shadowDiv.offsetLeft, 1);

	148 assert_approx_equals(window.scrollY, shadowDiv.offsetTop, 1);

	149 scrollShadowDomElementIntoView.done();

	150 });

	151 }

	152

	153 window.onload = function() {

	154 window.scrollTo(0, 0);
	foolip 2017/06/15 08:38:03 Is this because the page can scroll to the old pos Is this because the page can scroll to the old position when reloading the page manually? Can you add a comment? Since the tests are run one after the other, can you also assert the preconditions for each test? At the very least, I think they assume that they're not already at the target scroll position? sunyunjia 2017/06/15 18:00:26 Done. Show quoted text On 2017/06/15 08:38:03, foolip wrote: > Is this because the page can scroll to the old position when reloading the page > manually? Can you add a comment? > > Since the tests are run one after the other, can you also assert the > preconditions for each test? At the very least, I think they assume that they're > not already at the target scroll position? Done.
	155 animate(null, test1);

	156 }

	157

	158 </script>

OLD	NEW