Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2157133002:
Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests (Closed)
|
Created:
4 years, 5 months ago by lanwei Modified:
4 years, 3 months ago CC:
chromium-reviews, WRONG-USE-chromium, kenneth.christiansen, nzolghadr+blinkwatch_chromium.org, dshwang, eae+blinkwatch, slimming-paint-reviews_chromium.org, extensions-reviews_chromium.org, szager+layoutwatch_chromium.org, jam, darin-cc_chromium.org, jchaffraix+rendering, blink-reviews-paint_chromium.org, blink-reviews, chromium-apps-reviews_chromium.org, mlamouri+watch-content_chromium.org, blink-reviews-w3ctests_chromium.org, zoltan1, Peter Beverloo, blink-reviews-layout_chromium.org, piman+watch_chromium.org, jochen+watch_chromium.org, dtapuska+blinkwatch_chromium.org, mlamouri+watch-test-runner_chromium.org, tfarina, pdr+renderingwatchlist_chromium.org, leviw+renderwatch, dtapuska+chromiumwatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests.
On my local machine, all the pointer event tests are passing except some special cases.
BUG=627341
Committed: https://crrev.com/2e075b5e0779d239d0d379fa3b6a92cd7db4c183
Cr-Commit-Position: refs/heads/master@{#410276}
Patch Set 1 : pointer tests #Patch Set 2 : Update TestExpections #
Total comments: 13
Patch Set 3 : pointer tests #
Total comments: 8
Patch Set 4 : pointer tests #
Total comments: 4
Patch Set 5 : Use constant variable #Messages
Total messages: 69 (58 generated)
Description was changed from ========== scroll tests scroll tests pointer tests scrol not happen pointer tests pointer event tests pointer test pointer test pointer test BUG=627341 ========== to ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests BUG=627341 ==========
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #2 (id:60001) has been deleted
Description was changed from ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests BUG=627341 ========== to ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests. On my local machine, all the pointer event tests are passing except some special cases. BUG=627341 ==========
lanwei@chromium.org changed reviewers: + mustaq@chromium.org, nzolghadr@chromium.org, rbyers@chromium.org
https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html:47: //setTimeout(function() { I really think we do not need this timeout for the test, and it also hard to wait in the JS to simulate this user input.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:895: crbug.com/619060 [ Linux ] imported/wpt/pointerevents/pointerevent_touch-action-pan-x-pan-y_touch-manual.html [ Pass ] I don't know about this. But when you say "Pass" how should this be interpreted? Does it skip the Leak bots only somehow? https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:915: crbug.com/629723 imported/wpt/pointerevents/pointerevent_touch-action-inherit_child-auto-child-none_touch-manual.html [ Skip ] I just tested this on latest Canary on my Mac and it works. What do you see? Based on the test description you should try scrolling with touch (or maybe touch emulation on Chrome desktop) and nothing should scroll. Then you should scroll the page down to get to the "Complete button" and tap it for the test to finish. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html:47: //setTimeout(function() { On 2016/07/21 20:12:29, lanwei wrote: > I really think we do not need this timeout for the test, and it also hard to > wait in the JS to simulate this user input. I agree with you. But I was told we shouldn't change the tests locally as if someone else re-imports the tests from wpt repo these local changes will go away I believe. We can skip the test and similar ones altogether for now until we change this test in the upstream first. Does this sound good? https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-button-test_touch-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-button-test_touch-manual.html:33: <button id="testbutton">Test Button</button> The same here regarding the local changes to these test files. I would say just skip it for now until we post a wpt patch for this. I'm going to follow up on that. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-svg-test_touch-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-svg-test_touch-manual.html:26: <svg id="testsvg" width="555" height="555" style="touch-action: none; border: 4px double red;"> Same here. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:81: target.scrollIntoViewIfNeeded(); Does anything go wrong if we always scrollIntoViewIfNeeded regardless of the needScroll? Then we can remove this parameter altogether I guess.
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:100001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:120001) has been deleted
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:140001) has been deleted
Patchset #3 (id:160001) has been deleted
https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:895: crbug.com/619060 [ Linux ] imported/wpt/pointerevents/pointerevent_touch-action-pan-x-pan-y_touch-manual.html [ Pass ] On 2016/07/21 21:18:14, Navid Zolghadr wrote: > I don't know about this. But when you say "Pass" how should this be interpreted? > Does it skip the Leak bots only somehow? The leaking problem still exists, the reason I changed them to pass, I want to test them on the new change. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:915: crbug.com/629723 imported/wpt/pointerevents/pointerevent_touch-action-inherit_child-auto-child-none_touch-manual.html [ Skip ] On 2016/07/21 21:18:15, Navid Zolghadr wrote: > I just tested this on latest Canary on my Mac and it works. What do you see? > Based on the test description you should try scrolling with touch (or maybe > touch emulation on Chrome desktop) and nothing should scroll. Then you should > scroll the page down to get to the "Complete button" and tap it for the test to > finish. I will try them again. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_lostpointercapture_for_disconnected_node-manual.html:47: //setTimeout(function() { On 2016/07/21 21:18:15, Navid Zolghadr wrote: > On 2016/07/21 20:12:29, lanwei wrote: > > I really think we do not need this timeout for the test, and it also hard to > > wait in the JS to simulate this user input. > > I agree with you. But I was told we shouldn't change the tests locally as if > someone else re-imports the tests from wpt repo these local changes will go away > I believe. We can skip the test and similar ones altogether for now until we > change this test in the upstream first. Does this sound good? Yes. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-button-test_touch-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-button-test_touch-manual.html:33: <button id="testbutton">Test Button</button> On 2016/07/21 21:18:15, Navid Zolghadr wrote: > The same here regarding the local changes to these test files. I would say just > skip it for now until we post a wpt patch for this. I'm going to follow up on > that. Sure. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-svg-test_touch-manual.html (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt/pointerevents/pointerevent_touch-action-svg-test_touch-manual.html:26: <svg id="testsvg" width="555" height="555" style="touch-action: none; border: 4px double red;"> On 2016/07/21 21:18:15, Navid Zolghadr wrote: > Same here. Done. https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:81: target.scrollIntoViewIfNeeded(); On 2016/07/21 21:18:15, Navid Zolghadr wrote: > Does anything go wrong if we always scrollIntoViewIfNeeded regardless of the > needScroll? Then we can remove this parameter altogether I guess. I do not know why for some test pages, even I use scrollIntoViewIfNeeded, but it still scrolls, and the pages do not need to be scrolled for test.
https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:71: if (startY > windowHeight) { Can we use window.innerHeight instead of this and remove the constants? https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js:6: , scrollReturnInterval); I couldn't find where this value is defined. What is this? https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js:18: touchScrollByPosition(x, targetRect.top + 100, 30, "right", callback_function); This will be simplified when we add an id for the button. Right? Can you file a bug for this so that we remember to update this when the id gets added? Is that the case for the few other places? Can you just mention the crbug number in each so we can do a quick search and fix them all later. https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-table-test_touch-manual-automation.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-table-test_touch-manual-automation.js:16: touchSmoothScrollUp(cell3); I believe we can use touchScrollLeftInTarget and touchScrollUpInTarget in this test. Right?
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
rbyers@chromium.org changed reviewers: - rbyers@chromium.org
Patchset #4 (id:200001) has been deleted
https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:71: if (startY > windowHeight) { On 2016/07/29 16:30:39, Navid Zolghadr wrote: > Can we use window.innerHeight instead of this and remove the constants? Acknowledged. https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js:6: , scrollReturnInterval); On 2016/07/29 16:30:39, Navid Zolghadr wrote: > I couldn't find where this value is defined. What is this? It is defined in the tests where needs setTimeout. https://codesearch.chromium.org/chromium/src/third_party/WebKit/LayoutTests/i... https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-button-test_touch-manual-automation.js:18: touchScrollByPosition(x, targetRect.top + 100, 30, "right", callback_function); On 2016/07/29 16:30:39, Navid Zolghadr wrote: > This will be simplified when we add an id for the button. Right? Can you file a > bug for this so that we remember to update this when the id gets added? Is that > the case for the few other places? Can you just mention the crbug number in each > so we can do a quick search and fix them all later. Sure, I will file a bug for the need of ID for some tests. https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-table-test_touch-manual-automation.js (right): https://codereview.chromium.org/2157133002/diff/180001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_touch-action-table-test_touch-manual-automation.js:16: touchSmoothScrollUp(cell3); On 2016/07/29 16:30:39, Navid Zolghadr wrote: > I believe we can use touchScrollLeftInTarget and touchScrollUpInTarget in this > test. Right? Yes, Done.
Thanks. lgtm.
LGTM modulo nits on the js util readability... https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:80: chrome.gpuBenchmarking.smoothDrag(targetRect.left+10, targetRect.top+targetRect.height/2, targetRect.left+10, targetRect.top+10); Please replace the 5s and 10s with a single constant whose name should explain it. boundaryOffset? https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:90: chrome.gpuBenchmarking.smoothDrag(targetRect.left+targetRect.width/2, targetRect.top+10, targetRect.left+10, targetRect.top+10, callback_func); Are the scroll amounts arbitrary? I mean not really related to the height/width of the targetRect? If this is the case, please replace the parameters to some named constant like touchScrollOffset.
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #5 (id:240001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #5 (id:260001) has been deleted
The CQ bit was checked by lanwei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org, nzolghadr@chromium.org Link to the patchset: https://codereview.chromium.org/2157133002/#ps280001 (title: "Use constant variable")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests. On my local machine, all the pointer event tests are passing except some special cases. BUG=627341 ========== to ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests. On my local machine, all the pointer event tests are passing except some special cases. BUG=627341 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:280001)
Message was sent while issue was closed.
Description was changed from ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests. On my local machine, all the pointer event tests are passing except some special cases. BUG=627341 ========== to ========== Make gpuBenchmarking.smoothDrag or smoothScrollBy scroll properly in pointer event tests. On my local machine, all the pointer event tests are passing except some special cases. BUG=627341 Committed: https://crrev.com/2e075b5e0779d239d0d379fa3b6a92cd7db4c183 Cr-Commit-Position: refs/heads/master@{#410276} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/2e075b5e0779d239d0d379fa3b6a92cd7db4c183 Cr-Commit-Position: refs/heads/master@{#410276}
Message was sent while issue was closed.
https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:80: chrome.gpuBenchmarking.smoothDrag(targetRect.left+10, targetRect.top+targetRect.height/2, targetRect.left+10, targetRect.top+10); On 2016/08/04 19:08:37, mustaq wrote: > Please replace the 5s and 10s with a single constant whose name should explain > it. boundaryOffset? Done. https://codereview.chromium.org/2157133002/diff/220001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:90: chrome.gpuBenchmarking.smoothDrag(targetRect.left+targetRect.width/2, targetRect.top+10, targetRect.left+10, targetRect.top+10, callback_func); On 2016/08/04 19:08:37, mustaq wrote: > Are the scroll amounts arbitrary? I mean not really related to the height/width > of the targetRect? If this is the case, please replace the parameters to some > named constant like touchScrollOffset. Done. |
