Summary: | gesture event scrolling of iframe in overflow div scrolls wrong target | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Kroeger <rjkroege> | ||||
Component: | UI Events | Assignee: | Robert Kroeger <rjkroege> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | jamesr, rjkroege, tdanderson | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Robert Kroeger
2013-01-03 14:23:47 PST
Created attachment 181222 [details]
Patch
jamesr@: Please take a look? Comment on attachment 181222 [details]
Patch
I'm a little confused by the naming in here. Why would wheel events ever be latched?
(In reply to comment #3) > (From update of attachment 181222 [details]) > I'm a little confused by the naming in here. Why would wheel events ever be latched? AFAIK: the "latching" here arranges for all wheel events comprising a single touchscreen scroll gesture to be delivered to the same original node. Aside: I am not convinced that I like the pre-existing code here -- I think that https://bugs.webkit.org/show_bug.cgi?id=103952 is going to make it better -- but I wanted to fix http://crbug.com/167482 in a timely fashion. (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 181222 [details] [details]) > > I'm a little confused by the naming in here. Why would wheel events ever be latched? > > AFAIK: the "latching" here arranges for all wheel events comprising a single touchscreen scroll gesture to be delivered to the same original node. That doesn't make any sense - touchscreen interactions should not generate wheel events at all and touchpad interactions (which do generate wheels) should deliver events at a point, not at a node. > > Aside: I am not convinced that I like the pre-existing code here -- I think that https://bugs.webkit.org/show_bug.cgi?id=103952 is going to make it better -- but I wanted to fix http://crbug.com/167482 in a timely fashion. (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 181222 [details] [details] [details]) > > > I'm a little confused by the naming in here. Why would wheel events ever be latched? > > > > AFAIK: the "latching" here arranges for all wheel events comprising a single touchscreen scroll gesture to be delivered to the same original node. > > That doesn't make any sense - touchscreen interactions should not generate wheel events at all and touchpad interactions (which do generate wheels) should deliver events at a point, not at a node. You're right. http://crbug.com/134520 tracks in-progress work to fix. I had intended this patch to fix http://crbug.com/167482 in m25 but if this particular patch is pointless, perhaps we could discuss alternatives there? > > > > > Aside: I am not convinced that I like the pre-existing code here -- I think that https://bugs.webkit.org/show_bug.cgi?id=103952 is going to make it better -- but I wanted to fix http://crbug.com/167482 in a timely fashion. (In reply to comment #1) > Created an attachment (id=181222) [details] > Patch Note: I have included this change in my latest WIP patch for https://bugs.webkit.org/show_bug.cgi?id=103952 Comment on attachment 181222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181222&action=review > LayoutTests/fast/events/touch/gesture/resources/scroll-inside-editable-iframe.html:9 > + margin: 0px;a should that 'a' be there? > LayoutTests/fast/events/touch/gesture/resources/scroll-inside-editable-iframe.html:35 > +<body> </body> Comment on attachment 181222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181222&action=review > LayoutTests/fast/events/touch/gesture/touch-gesture-scroll-iframe-editable.html:75 > + touchtarget.contentDocument.body.addEventListener("mousewheel", recordWheel); Why do you add the event listener to contentDocument for a scroll, but to contentDocument.body for a mousewheel? Shouldn't these both be set on contentDocument.body? Was fixed properly via other changes. Comment on attachment 181222 [details] Patch Cleared review? from attachment 181222 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). |