[webkit-reviews] review denied: [Bug 17589] Scroll wheel
sensitivity ignored on Vista : [Attachment 20747] patch attempt
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 22 07:22:40 PDT 2008
Adam Roben (aroben) <aroben at apple.com> has denied Marcus <gamander at gmail.com>'s
request for review:
Bug 17589: Scroll wheel sensitivity ignored on Vista
http://bugs.webkit.org/show_bug.cgi?id=17589
Attachment 20747: patch attempt
http://bugs.webkit.org/attachment.cgi?id=20747&action=edit
------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
Thanks for the patch! This is a good step in the right direction.
This patch won't fix the issue for overflow areas (e.g., <div style="overflow:
scroll">). Overflow areas use RenderLayer::scroll to do their scrolling. We'll
want to fix both at the same time, and hopefully in a way that avoids
duplicating the code you've added.
+ if (!::SystemParametersInfo(SPI_GETWHEELSCROLLLINES, 0,
&linesToScroll, 0))
On Vista there's also an SPI_GETWHEELSCROLLCHARS value that we should support
for horizontal scrolling. Since this value isn't present in pre-Vista headers,
you'll have to declare it at the top of this file, like this:
static const UINT SPI_GETWHEELSCROLLCHARS = <the value that Vista uses>;
+ linesToScroll = 3; // Windows default value
This should be placed into a named constant at the top of the file, like this:
static const unsigned defaultLinesToScroll = 3;
+ // Like Mozilla, use default font to determine the number of
pixels per line.
+ const FrameView* frameView = static_cast<const FrameView*>(this);
+ const Frame* frame = frameView->frame();
+ int lineStep;
+ if (frame && frame->contentRenderer() &&
frame->contentRenderer()->style())
+ lineStep = linesToScroll *
frame->contentRenderer()->style()->font().lineSpacing();
+ else
+ lineStep = linesToScroll * LINE_STEP / 2; // fallback value
+
+ scrollBy(-e.deltaX() * lineStep, -e.deltaY() * lineStep);
This is a layering violation. Classes in platform/ should not in general be
aware of classes outside of platform/, so it seems unfortunate to be
introducing RenderView usage here. We'd also want this change to be made on all
platforms, not just Windows, I believe. This change seems logically distinct
from supporting the system preferences for scrolling amount, so this should be
turned into a separate patch.
More information about the webkit-reviews
mailing list