Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1898333002:
PDF scrolling with wheel gestures (Closed)
|
Created:
4 years, 8 months ago by dtapuska Modified:
4 years, 8 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, dtapuska+chromiumwatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPDF scrolling with wheel gestures
Scroll gestures may be generated twice, as a plugin
may 'resend' them if not fully consumed by the plugin,
and they may be generated once again inside the main
renderer host. Re-sending of gesture scroll events from
the plugin causes double scroll values in PDFs
This was more noticable on Windows which uses a larger tick counter (120) vs
53 on Linux.
Fix is to not generate gestures inside the plugin container.
BUG=604734
Committed: https://crrev.com/874feac6c0e08aab52bd08fe33d5daee0ae07fa5
Cr-Commit-Position: refs/heads/master@{#388508}
Patch Set 1 #
Messages
Total messages: 15 (6 generated)
Description was changed from ========== PDF scrolling with wheel gestures Gestures could would be generated twice; once inside the plugin container host and once inside the main renderer host. With events redispacted for gesture events this would cause double scroll values in PDFs. This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ========== to ========== PDF scrolling with wheel gestures Gestures could would be generated twice; once inside the plugin container host and once inside the main renderer host. With events redispacted for gesture events this would cause double scroll values in PDFs. This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ==========
dtapuska@chromium.org changed reviewers: + tdresser@chromium.org, wjmaclean@chromium.org
LGTM
Any idea how hard it would be to add a test for this?
On 2016/04/20 at 12:08:18, tdresser wrote: > Any idea how hard it would be to add a test for this? Also LGTM, though the issue description needs cleaning up: "could would", "redispacted" Perhaps something like: (but feel free to modify) "Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs."
Description was changed from ========== PDF scrolling with wheel gestures Gestures could would be generated twice; once inside the plugin container host and once inside the main renderer host. With events redispacted for gesture events this would cause double scroll values in PDFs. This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ========== to ========== PDF scrolling with wheel gestures Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ==========
On 2016/04/20 12:46:35, wjmaclean wrote: > On 2016/04/20 at 12:08:18, tdresser wrote: > > Any idea how hard it would be to add a test for this? > > Also LGTM, though the issue description needs cleaning up: > "could would", "redispacted" > > Perhaps something like: (but feel free to modify) > > "Scroll gestures may be generated twice, as a plugin > may 'resend' them if not fully consumed by the plugin, > and they may be generated once again inside the main > renderer host. Re-sending of gesture scroll events from > the plugin causes double scroll values in PDFs." James do you have an idea where I could write a test that ensures this doesn't regress?
The CQ bit was checked by dtapuska@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1898333002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1898333002/1
On 2016/04/20 at 13:45:02, dtapuska wrote: > On 2016/04/20 12:46:35, wjmaclean wrote: > > On 2016/04/20 at 12:08:18, tdresser wrote: > > > Any idea how hard it would be to add a test for this? > > > > Also LGTM, though the issue description needs cleaning up: > > "could would", "redispacted" > > > > Perhaps something like: (but feel free to modify) > > > > "Scroll gestures may be generated twice, as a plugin > > may 'resend' them if not fully consumed by the plugin, > > and they may be generated once again inside the main > > renderer host. Re-sending of gesture scroll events from > > the plugin causes double scroll values in PDFs." > > James do you have an idea where I could write a test that ensures this doesn't regress? Perhaps re-enabling WebViewGuestScrollTest.DISABLED_TestGuestWheelScrollsBubble in web_view_browsertest.cc would suffice? Not sure why this test went flaky when the touch-version didn't, but perhaps your CL will fix it?
Message was sent while issue was closed.
Description was changed from ========== PDF scrolling with wheel gestures Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ========== to ========== PDF scrolling with wheel gestures Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== PDF scrolling with wheel gestures Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 ========== to ========== PDF scrolling with wheel gestures Scroll gestures may be generated twice, as a plugin may 'resend' them if not fully consumed by the plugin, and they may be generated once again inside the main renderer host. Re-sending of gesture scroll events from the plugin causes double scroll values in PDFs This was more noticable on Windows which uses a larger tick counter (120) vs 53 on Linux. Fix is to not generate gestures inside the plugin container. BUG=604734 Committed: https://crrev.com/874feac6c0e08aab52bd08fe33d5daee0ae07fa5 Cr-Commit-Position: refs/heads/master@{#388508} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/874feac6c0e08aab52bd08fe33d5daee0ae07fa5 Cr-Commit-Position: refs/heads/master@{#388508} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
