Bug 64336 - RenderListBox scroller doesn't scroll
Summary: RenderListBox scroller doesn't scroll
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-11 18:46 PDT by Joseph Pecoraro
Modified: 2012-04-19 16:36 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.48 KB, patch)
2011-07-11 19:59 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (1.58 KB, patch)
2011-07-11 20:04 PDT, Joseph Pecoraro
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2011-07-11 18:46:44 PDT
Existing test:
LayoutTests/platform/mac/fast/forms/listbox-scrollbar-hit-test.html

The test seems to succeed in DRT but the scrollbars don't work properly in Safari.
Scrollwheel events seem to work, but clicking on the scrollbar, using up/down etc
all don't work.
Comment 1 Joseph Pecoraro 2011-07-11 18:47:23 PDT
Hmm, I see the "native animator" getting called, but ends up doing nothing:

    ScrollAnimatorMac::scroll...
    [m_scrollAnimationHelper.get() scrollToPoint:newPoint];

    gdb> po m_scrollAnimationHelper.get()
    <NSScrollAnimationHelper: 0x118616b50>

Because ScrollAnimatorMac::adjustScrollPositionIfNecessary resets the y scroll to 0:

    float newY = max<float>(min<float>(position.y(), m_scrollableArea->contentsSize().height() - m_scrollableArea->visibleHeight()), 0);
    => max(min(1, 125-129), 0)
    => max(min(1, -4), 0)
    => max(-1, 0)
    => 0

Is the "- m_scrollableArea->visibleHeight()" part of the equation necessary? It sounds like
we just want to clamp in the content size, in this case from 0 to 125 and the visible height
is not needed?

However, I think custom scrollers work. So I'll compare this to custom scrollers.
Comment 2 Joseph Pecoraro 2011-07-11 18:54:00 PDT
This breaks whenever the visibleHeight is greater than the contentHeight.
So this is broken for custom scrollbars as well.
Comment 3 Joseph Pecoraro 2011-07-11 19:59:14 PDT
Created attachment 100430 [details]
[PATCH] Proposed Fix
Comment 4 Joseph Pecoraro 2011-07-11 20:04:58 PDT
Created attachment 100431 [details]
[PATCH] Proposed Fix

Now with git add ChangeLog!
Comment 5 Sam Weinig 2011-07-12 20:38:47 PDT
Joe, do you know why this seems to work in DRT?
Comment 6 Joseph Pecoraro 2011-07-12 20:46:38 PDT
Nope, I didn't look into that =/.
Comment 7 Eric Seidel (no email) 2012-02-16 13:59:16 PST
The difference between content and visible should be padding/margin, no?  We need some way to test this...
Comment 8 Adele Peterson 2012-04-19 16:28:31 PDT
Would a test that used up/down work?
Comment 9 Eric Seidel (no email) 2012-04-19 16:36:07 PDT
Comment on attachment 100431 [details]
[PATCH] Proposed Fix

r- for lack of test or lack of why testing is impossible/impratical.