Summary: | Can't scroll page downwards with scroll wheel, when pointer is on top of non-scrolling iframe | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | jonathanjohnsson | ||||||||||||
Component: | Frames | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ap, hyatt, mitz, mjuhos, mrowe, timothy.c.bates | ||||||||||||
Priority: | P3 | Keywords: | HasReduction | ||||||||||||
Version: | 523.x (Safari 3) | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.4 | ||||||||||||||
URL: | http://www.screwedbydesign.com/blog/2006/08/firefox_2_visual_refresh_progress.php | ||||||||||||||
Attachments: |
|
Description
jonathanjohnsson
2006-08-05 02:52:16 PDT
Created attachment 9889 [details]
Iframe used in reduction
Created attachment 9890 [details]
Reduction of the reported page
Repros in the current build. can't scroll down when mouse is over the iFrame. 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...
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. 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. 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... *** Bug 13586 has been marked as a duplicate of this bug. *** Dave, any luck with finding a workaround? 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. 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 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 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).
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.
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. (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. Actually it's not equivalent in either case, and is indeed worse if AppKit will swallow the event. 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. (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 on attachment 15412 [details]
Forward wheel events to the next responder if scrolling is not allowed
I'm convinced.
r=me
Landed in r24050. 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. 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. 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. (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). |