Bug 106040

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Robert Kroeger

Reported 2013-01-03 14:23:47 PST

When a scrollable iframe is placed in a fixed size overflow-scroll div, the (synthetic) wheel events used to implement the gesture scroll are dispatched on the overflow div rather than the iframe. As a result the contents of the iframe cannot be scrolled with gesture events.

Attachments
Patch (7.71 KB, patch) 2013-01-03 14:36 PST, Robert Kroeger	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Robert Kroeger

Comment 1 2013-01-03 14:36:01 PST

Created attachment 181222 [details] Patch

Robert Kroeger

Comment 2 2013-01-03 14:39:21 PST

jamesr@: Please take a look?

James Robinson

Comment 3 2013-01-03 16:04:44 PST

Comment on attachment 181222 [details] Patch I'm a little confused by the naming in here. Why would wheel events ever be latched?

Robert Kroeger

Comment 4 2013-01-04 08:21:07 PST

(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.

James Robinson

Comment 5 2013-01-04 10:25:37 PST

(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.

Robert Kroeger

Comment 6 2013-01-04 12:58:25 PST

(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.

Terry Anderson

Comment 7 2013-01-08 13:05:49 PST

(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

Terry Anderson

Comment 8 2013-01-08 16:52:43 PST

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>

Terry Anderson

Comment 9 2013-01-16 12:43:35 PST

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?

Robert Kroeger

Comment 10 2013-02-04 12:50:36 PST

Was fixed properly via other changes.

Eric Seidel (no email)

Comment 11 2013-03-01 02:49:15 PST

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).

Note You need to log in before you can comment on or make changes to this bug.