Bug 81938 – [Qt] Scrolling with mouse wheel does not work in MiniBrowser with --desktop

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Platform

[Balazs Kelemen]

WebEventFactoryQt.cpp is using QApplication::wheelScrollLines() which is wrong since we don't instantiate a QApplication.

[Balazs Kelemen]

Created attachment 133301 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 133301 [details]
Patch

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

> Source/WebKit2/Shared/qt/WebEventFactoryQt.cpp:154
> -    deltaX *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> -    deltaY *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> +    static const int cWheelScrollLines = 3;
> +    deltaX *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;
> +    deltaY *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;

Make sure to keep the same code in WebCore/platform/qt/WheelEventQt.cpp up to date.

But is there any good reason not to use the input delta value instead of QApplication::wheelScrollLines()? The value of a wheel-event should always either be the lines or pixels to be scrolled, and this can change if multiple wheel-events have been coalezed for instance.

[Balazs Kelemen]

(In reply to comment #2)
> (From update of attachment 133301 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133301&action=review
> 
> > Source/WebKit2/Shared/qt/WebEventFactoryQt.cpp:154
> > -    deltaX *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> > -    deltaY *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> > +    static const int cWheelScrollLines = 3;
> > +    deltaX *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;
> > +    deltaY *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;
> 
> Make sure to keep the same code in WebCore/platform/qt/WheelEventQt.cpp up to date.
> 
> But is there any good reason not to use the input delta value instead of QApplication::wheelScrollLines()? The value of a wheel-event should always either be the lines or pixels to be scrolled, and this can change if multiple wheel-events have been coalezed for instance.

I don't really know the history of this logic, so I'm just going to explain how I understand it. In the case of a fine-resolution device we use the delta as it is. In the case of fulltick device we need to compute the pixel values because WebCore expects that. I don't think that it can change: we either get fine-resolution or fulltick values but never both, at least for one device. So I think the logic we have here is necessary and correct.

[Balazs Kelemen]

Created attachment 133470 [details]
Patch

[Balazs Kelemen]

(In reply to comment #4)
> Created an attachment (id=133470) [details]
> Patch

Applied the change to WheelEventQt.cpp as well.

[Balazs Kelemen]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 133301 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=133301&action=review
> > 
> > > Source/WebKit2/Shared/qt/WebEventFactoryQt.cpp:154
> > > -    deltaX *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> > > -    deltaY *= (fullTick) ? QApplication::wheelScrollLines() * cDefaultQtScrollStep : 1;
> > > +    static const int cWheelScrollLines = 3;
> > > +    deltaX *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;
> > > +    deltaY *= (fullTick) ? cWheelScrollLines * cDefaultQtScrollStep : 1;
> > 
> > Make sure to keep the same code in WebCore/platform/qt/WheelEventQt.cpp up to date.
> > 
> > But is there any good reason not to use the input delta value instead of QApplication::wheelScrollLines()? The value of a wheel-event should always either be the lines or pixels to be scrolled, and this can change if multiple wheel-events have been coalezed for instance.
> 
> I don't really know the history of this logic, so I'm just going to explain how I understand it. In the case of a fine-resolution device we use the delta as it is. In the case of fulltick device we need to compute the pixel values because WebCore expects that. I don't think that it can change: we either get fine-resolution or fulltick values but never both, at least for one device. So I think the logic we have here is necessary and correct.

Hmm, thinking it over again, now I don't see the evidence why we need to compute another delta with these constants. The bug that the comment is referring to is about fixing behavior for yahoo.com. It does some strange with the wheel events. Anyway, for now I think it's better to check in this typo fix and keep the behavior, later we can take a try with simplifying the logic.

[Balazs Kelemen]

Comment on attachment 133470 [details]
Patch

I realized that we don't even build WheelEventQt.cpp. Seems like it has just left over. So, the firts patch is sufficient.

[Simon Hausmann]

Comment on attachment 133301 [details]
Patch

A similar change was applied in r112093 - clearing review :)

[Simon Hausmann]

*** This bug has been marked as a duplicate of bug 79458 ***

[Allan Sandfeld Jensen]

(In reply to comment #9)
> 
> *** This bug has been marked as a duplicate of bug 79458 ***

Fix compilation with QtWidget? What does that have to do with this bug?

[Allan Sandfeld Jensen]

(In reply to comment #10)
> (In reply to comment #9)
> > 
> > *** This bug has been marked as a duplicate of bug 79458 ***
> 
> Fix compilation with QtWidget? What does that have to do with this bug?

Nevermind. I found it.