Bug 10267 - Can't scroll page downwards with scroll wheel, when pointer is on top of non-scrolling iframe
Summary: Can't scroll page downwards with scroll wheel, when pointer is on top of non-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P3 Normal
Assignee: Nobody
URL: http://www.screwedbydesign.com/blog/2...
Keywords: HasReduction
: 13586 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-08-05 02:52 PDT by jonathanjohnsson
Modified: 2007-07-06 12:25 PDT (History)
6 users (show)

See Also:


Attachments
Iframe used in reduction (128 bytes, text/html)
2006-08-05 02:55 PDT, jonathanjohnsson
no flags Details
Reduction of the reported page (693 bytes, text/html)
2006-08-05 02:56 PDT, jonathanjohnsson
no flags Details
Reduction of the reported page (678 bytes, text/html)
2007-01-13 06:53 PST, jonathanjohnsson
no flags Details
Forward wheel events to the next responder if scrolling is not allowed (1.76 KB, patch)
2007-07-05 12:22 PDT, mitz
mjs: review-
Details | Formatted Diff | Diff
Forward wheel events to the next responder if scrolling is not allowed (1.73 KB, patch)
2007-07-05 23:43 PDT, mitz
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jonathanjohnsson 2006-08-05 02:52:16 PDT
In the reported page, try scrolling the page with the mouse scroll wheel, when the mouse pointer is on top of the google ad just above the comments section. You can only scroll up, not down. (The ads may change, it's the two column, text only ads that seem to have this behaviour.)

I reduced the problem, and it shows that if the iframe has scrolling="no" set, and the iframe height is smaller than its enclosed source, you can't scroll downward as described above (scrolling sideways works).

Both Firefox 1.5 and Opera 9 will scroll in any direction, while both latest Safari and latest WebKit won't scroll downwards.

Reduction follows.
Comment 1 jonathanjohnsson 2006-08-05 02:55:20 PDT
Created attachment 9889 [details]
Iframe used in reduction
Comment 2 jonathanjohnsson 2006-08-05 02:56:52 PDT
Created attachment 9890 [details]
Reduction of the reported page
Comment 3 tim bates 2006-12-07 18:20:50 PST
Repros in the current build. can't scroll down when mouse is over the iFrame.
Comment 4 jonathanjohnsson 2007-01-13 06:53:55 PST
Created attachment 12415 [details]
Reduction of the reported page

Changed a link in the reduction, the link broke when buzilla moved. Maybe it isn't a good idea to link between attachments...
Comment 5 jonathanjohnsson 2007-01-13 06:56:42 PST
Trying the reduction using a trackpad with ToT, I noticed that you can neither scroll down nor to the right when the mouse pointer is on top of the iframe.
Comment 6 jonathanjohnsson 2007-01-13 07:05:05 PST
I just want to say that this is one of very few bugs that I notice almost daily, and that I would really appreciate it if someone could fix it, even if it's non-critical and easy to work around.
Comment 7 Dave Hyatt 2007-01-13 14:35:45 PST
This appears to be an AppKit bug.  It is not forwarding the scroll wheel event up the nextResponder chain properly.  For some reason it's just eating the event.  I'll have to hunt for a workaround...
Comment 8 mitz 2007-05-04 11:06:19 PDT
*** Bug 13586 has been marked as a duplicate of this bug. ***
Comment 9 Mark Rowe (bdash) 2007-07-04 09:53:03 PDT
Dave, any luck with finding a workaround?
Comment 10 Mark Rowe (bdash) 2007-07-05 10:41:45 PDT
NSScrollView determines whether to forward the event to the next responder by verifying whether the document view is already "pinned" at the relevant edge of the scroll view.  In the case of the attached reduction this is never true.  The iframe's document is 1 pixel taller than the iframe itself, but it has the scrollbars hidden.  Because the document view is not pinned the scroll view accepts the mouse wheel event, but as the scrollbars are hidden the scrolling has no effect.
Comment 11 mitz 2007-07-05 12:22:33 PDT
Created attachment 15403 [details]
Forward wheel events to the next responder if scrolling is not allowed

Patch mostly by Mark Rowe.

I did not find a way in AppKit to generate mouse wheel events, so there is no test (and no changes to DumpRenderTree).
Comment 12 mitz 2007-07-05 12:49:39 PDT
Comment on attachment 15403 [details]
Forward wheel events to the next responder if scrolling is not allowed

Forgot to mention that checking each direction separately is required for cases such as an iframe whose root has 'overflow-x:hidden; overflow-y:scroll'.
Comment 13 Maciej Stachowiak 2007-07-05 19:38:32 PDT
Comment on attachment 15403 [details]
Forward wheel events to the next responder if scrolling is not allowed

This looks incorrect in the case where both deltas are non-zero - in that case it should be checking for both horizontal and vertical scrolling allowed.

(I think this can actually happen when doing the "two finger scroll" gesture on trackpacks).
Comment 14 mitz 2007-07-05 23:43:49 PDT
Created attachment 15412 [details]
Forward wheel events to the next responder if scrolling is not allowed

This version decides based on the bigger component of the scroll direction.
Comment 15 Mark Rowe (bdash) 2007-07-06 00:01:39 PDT
The reason whether the Y axis was given priority over the X axis in the initial patch as this is the same logic that NSScrollView uses to determine whether a the document view is pinned.  If the scroll event has *any* Y component it will check whether it is pinned on the Y axis, and completely ignore the X component.  In my opinion it makes sense to match that logic here.
Comment 16 mitz 2007-07-06 00:14:26 PDT
(In reply to comment #15)
> If the scroll event has *any* Y component it will
> check whether it is pinned on the Y axis, and completely ignore the X
> component.

In that case, if the view is not pinned on the Y axis, but does not have a vertical scrollbar, is the event completely ignored or does horizontal scrolling still happen if possible? If it's the former, then I agree that it's best to match AppKit. Otherwise, I think the new patch is currently equivalent but will be even better in the future if AppKit changes.
Comment 17 mitz 2007-07-06 00:19:21 PDT
Actually it's not equivalent in either case, and is indeed worse if AppKit will swallow the event.
Comment 18 Mark Rowe (bdash) 2007-07-06 02:03:58 PDT
In the scenario you mention, the Y component of the scroll event will simply be ignored and the scroll view will process the X component.
Comment 19 mitz 2007-07-06 02:08:48 PDT
(In reply to comment #18)
> In the scenario you mention, the Y component of the scroll event will simply be
> ignored and the scroll view will process the X component.
> 

So if the iframe is scrollable along the X axis and a wheel event comes in with a tiny Y component and a big X component, the new patch will keep the event in the iframe and subsequently it will scroll horizontally, whereas the initial patch will forward to the page (which may or may not scroll). I think the behavior of the new patch is better.
Comment 20 Maciej Stachowiak 2007-07-06 02:47:46 PDT
Comment on attachment 15412 [details]
Forward wheel events to the next responder if scrolling is not allowed

I'm convinced.

r=me
Comment 21 Mark Rowe (bdash) 2007-07-06 02:54:28 PDT
Landed in r24050.
Comment 22 jonathanjohnsson 2007-07-06 04:41:05 PDT
I tested the reduction above using ToT (r24057), and it now works to two-finger scroll in all directions when on top of the black area. Nice, thanks! :) 

However, when the pointer is positioned on what seems to be a two pixel thick border around the iframe, I can't scroll in any direction. While it in no way is as annoying as before, and the target area is small, I believe you could quite easily stumble upon it when two-finger scrolling, since this is done pixel by pixel. Try two-finger scrolling slowly across the reduction to see what I mean.
Comment 23 Mark Rowe (bdash) 2007-07-06 04:46:57 PDT
Thanks for testing Jonathan.  The other problem you describe is one we ran into while testing.  It'd be great if you could file a bug report specifically for that problem so it can also be tracked in Bugzilla.
Comment 24 jonathanjohnsson 2007-07-06 04:56:24 PDT
Ok, Mark, I will do that. I thought this new bug might have been a result of the fix for this bug, but now when I look at it I see that it appeared before the fix as well.
Comment 25 mitz 2007-07-06 12:25:24 PDT
(In reply to comment #24)
> Ok, Mark, I will do that. I thought this new bug might have been a result of
> the fix for this bug, but now when I look at it I see that it appeared before
> the fix as well.
> 

I think I have a fix for it, so I am going to file it now (it was a totally different problem than I thought).