REGRESSION (r163018): Can’t scroll in <select> lists

• Component: UI Events
• Status: RESOLVED FIXED

To reproduce: navigate to the URL and try to scroll the Component version.

<rdar://problem/15966438>

[Brent Fulgham]

I just reverted http://trac.webkit.org/changeset/162985 and retested. I continue to see the <select> problem.

I think these bugs are related, but I no longer think http://trac.webkit.org/changeset/162985 caused this regression.

Indeed, further testing shows that this was caused by <http://trac.webkit.org/r163018>, the fix for bug 123886.

[Jon Lee]

Will this issue be looked at very soon? Otherwise we should roll the patch out.

[Radu Stavila]

I'll be looking at it today.

[Radu Stavila]

Created attachment 222988 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 222988 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        This test is for the moment added as a ImageOnlyFailure expectation
> +        because the current implementation of eventSender cannot simulate mouse wheel events.

I'm not sure if it's worth it having a test that fails on all platforms.

What is the specific issue in EventSender? Linking to bug 42194 is not helpful in this regard. Mac DumpRenderTree has the code to send wheel events, so this seems like an overly broad statement that eventSender cannot simulate mouse wheel events.

[Brent Fulgham]

(In reply to comment #7)
> What is the specific issue in EventSender? Linking to bug 42194 is not helpful in this regard. Mac DumpRenderTree has the code to send wheel events, so this seems like an overly broad statement that eventSender cannot simulate mouse wheel events.

I've been wrestling with this issue the past couple of days. I have filed https://bugs.webkit.org/show_bug.cgi?id=127606 to extend eventSender to support wheel events better.

I do agree that the current infrastructure can send wheel events, but it is not currently capable of supporting gestures (such as a "flick" of the mouse wheel) that involve multiple mouse wheel phases and momentum support.

I'm not sure if those shortcomings are relevant to this particular test case.

For example, it might be possible to use the 'scrollTop' property of the scrollable region to confirm that the mouse operations caused the region to scroll.

[Brent Fulgham]

Comment on attachment 222988 [details]
Patch

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

> LayoutTests/fast/scrolling/scroll-select-list.html:9
> +

Could you do something like:

var startPosition = selectList.scrollTop;

> LayoutTests/fast/scrolling/scroll-select-list.html:13
> +      eventSender.mouseMoveTo(0, 0);

var endPosition = selectList.scrollTop;
... followed by:

if (startPosition != endPosition)
    testPassed("<select> region scrolled.");
else
    testFailed("<select> region did NOT scroll.");

[Darin Adler]

Comment on attachment 222988 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.h:455
> +    virtual bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1, Element** stopElement = 0, RenderBox* startBox = 0, const IntPoint& wheelEventAbsolutePoint = IntPoint());

nullptr instead of 0, please

> Source/WebCore/rendering/RenderListBox.h:77
> +    virtual bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1, Element** stopElement = 0, RenderBox* startBox = 0, const IntPoint& wheelEventAbsolutePoint = IntPoint()) override;

nullptr instead of 0 please

> Source/WebCore/rendering/RenderTextControlSingleLine.h:72
> +    virtual bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1, Element** stopElement = 0, RenderBox* startBox = 0, const IntPoint& wheelEventAbsolutePoint = IntPoint()) override final;

nullptr instead of 0 please

[Simon Fraser (smfr)]

Comment on attachment 222988 [details]
Patch

r=me but please address the comments and make a test if possible

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/163329.

[Brent Fulgham]

Note that better testing is being planned in Bug 127606.