Clicking where the overlay scroll track would be should not scroll the page

• Component: New Bugs
• Status: RESOLVED FIXED

[Mark Mentovai]

The overlay scrollbar eats clicks on Lion, even when the scrollbar is not visible. The user will place the mouse over web content such as a link, which is all that is visible at a certain position, and click, expecting the web content to receive the click. Instead, the scrollbar picks up the click, either swallowing it or scrolling the page. When the click is swallowed (because it occurred over where the scroll thumb would be), the scrollbar thumb and track don’t even appear, so the user is left without any feedback as to why the click didn’t reach the web content.

I experience this bug in both Safari 5.1.1 (7534.51.22) and Chrome 17.0.921.3 canary on Mac OS X 10.7.2 11C74.

Steps to Reproduce:

0. Use Safari or Chrome on a system that uses overlay scrollbars: a Lion system with a gestureable pointing device such as a laptop trackpad is required.
1. Visit a web page that can scroll vertically, such as http://www.nytimes.com/
2. Resize the window so that the web page is vertically scrollable and there are links present in the web content area at the extreme right edge of the window.
3. Wait for the overlays to disappear, if shown. This should only take a couple of seconds with the mouse cursor outside of the scrollbar area.
4. Click on a web link at the extreme right edge of the window, within the rightmost several pixels.

Expect: to navigate to the link that was clicked. The mouse cursor will have changed to a finger pointing, indicating that a click would navigate to the link.

Observe: Although the mouse cursor changed to the finger, the click does not navigate. Instead, WebKit behaves as though the scrollbar was clicked, even though the scrollbar was not visible.

If the click occurred over where the scroll thumb would have been, no visible feedback is provided at all, because the page does not scroll at all upon a single click of a scroll thumb. (The thumb, is, however, draggable.)

If the click occurred in the area where the scroll track would be but not over the thumb, web content jumps one page in the appropriate direction, and the scroll thumb appears.

[Radar WebKit Bug Importer]

<rdar://problem/10373860>

[Beth Dakin]

(In reply to comment #1)
> <rdar://problem/10373860>

That bug has been marked a duplicate of this one: <rdar://problem/9585424>

[Beth Dakin]

Created attachment 122329 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 122329 [details]
Patch

Do we need the same check in BuiltinPDFView?

[Beth Dakin]

(In reply to comment #4)
> (From update of attachment 122329 [details])
> Do we need the same check in BuiltinPDFView?

Oh, thank you for the reminder! We probably do. I will look into that now.

[Beth Dakin]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 122329 [details] [details])
> > Do we need the same check in BuiltinPDFView?
> 
> Oh, thank you for the reminder! We probably do. I will look into that now.

Looks like PDFs don't have this bug, actually.

[Beth Dakin]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 122329 [details] [details] [details])
> > > Do we need the same check in BuiltinPDFView?
> > 
> > Oh, thank you for the reminder! We probably do. I will look into that now.
> 
> Looks like PDFs don't have this bug, actually.

Oh, I take it back! Alexey sent me a test case that does show the bug.

[Beth Dakin]

Comment on attachment 122329 [details]
Patch

I will post a new patch shortly that fixes PDF too. If anyone can suggest a different name for Scrollbar::isCurrentlyVisible(), I'd love to hear it. Scrollbar's parent class Widget already has a function called isVisible() that has a different meaning, so it seems wrong to re-use that.

[Beth Dakin]

Created attachment 122479 [details]
Patch that fixes PDF as well

[Alexey Proskuryakov]

Comment on attachment 122479 [details]
Patch that fixes PDF as well

View in context: https://bugs.webkit.org/attachment.cgi?id=122479&action=review

> Source/WebCore/platform/Scrollbar.cpp:421
> +    if (!isClickable())
> +        return false;

If I'm reading it correctly, clicks in invisible scroll bars will not be handled by underlying PDF content. That's not a problem now, given that we don't support any links or forms in BuiltinPDFView, but we need to make sure that those work correctly eventually. That would mean that hit testing should return PluginView, not the Scrollbar.

[Beth Dakin]

Created attachment 122494 [details]
Better patch that also fixed PDF

You're right, Alexey. I think I got it this time!

[Alexey Proskuryakov]

Comment on attachment 122494 [details]
Better patch that also fixed PDF

The "clickable" concept could be somewhat confusing - is a visible, but disabled scrollbar clickable? Looks like the code uses both meanings in different places.

Then again, I'm not sure what it means for a scrollbar to be disabled.

[Beth Dakin]

Thanks for the review!

(In reply to comment #12)
> (From update of attachment 122494 [details])
> The "clickable" concept could be somewhat confusing - is a visible, but disabled scrollbar clickable? Looks like the code uses both meanings in different places.
> 
> Then again, I'm not sure what it means for a scrollbar to be disabled.

Hmm, this is a good point. I suppose isHitTestable would be more accurate, but that is not great either because it sounds a bit clumsy. I will think about it a bit more before I check in.

[Beth Dakin]

Committed with http://trac.webkit.org/changeset/105006

And I changed the name of isClickable() to shouldParticipateInHitTesting() as Alexey suggested on irc.