Bug 118019 – Change event should not be dispatched by clicking a scrollbar of select listbox

• Component: Forms
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Consider merging https://chromium.googlesource.com/chromium/blink/+/492549b0fcaa58a85aa0797446b62985a263704f

We fixed crbug.com/244406, but the fix was incomplete.

Bug detail:
If the size of option list is changed after the last 'change' event and
'mouseup' happens on the scrollbar, new 'change' event is dispatched even though
the selection state is not changed.
HTMLSelectElement::listBoxOnChange dispatches 'change' event if
m_lastOnChangeSelection.size() is different from the option list size. (Why?)

How to fix:
When the option list size is changed, we should update m_lastOnChangeSelection
so that it represents reality or should avoid to call listBoxOnChange() in
mouseup event handler. This patch implements the latter by clearing
m_lastOnChangeSelection.

[Radar WebKit Bug Importer]

<rdar://problem/14297760>

[Ruth Fong]

Created attachment 205753 [details]
Patch

[Ruth Fong]

Created attachment 205754 [details]
test file

With patch 205753 alone, I haven't been able to repro the issue of a change event firing after a change event and a change to size of the select list. (I click an option, then the button, and then the scrollbar and can't get the onChange event to fire).

Yet, when I run Chromium's layout test, I can hit the case and fail at line 47 with changeEventCounter = 2.  (https://chromium.googlesource.com/chromium/blink/+/492549b0fcaa58a85aa0797446b62985a263704f/LayoutTests/fast/forms/select/listbox-click-on-scrollbar.html)

Any ideas how to manually repro this situation?

[Kent Tamura]

(In reply to comment #3)
> Yet, when I run Chromium's layout test, I can hit the case and fail at line 47 with changeEventCounter = 2.  (https://chromium.googlesource.com/chromium/blink/+/492549b0fcaa58a85aa0797446b62985a263704f/LayoutTests/fast/forms/select/listbox-click-on-scrollbar.html)

You need to merge http://src.chromium.org/viewvc/blink?view=revision&revision=152868 too.

[Ruth Fong]

Created attachment 205841 [details]
Test file to repro size change bug

[Ruth Fong]

Created attachment 205843 [details]
Patch

[Build Bot]

Comment on attachment 205843 [details]
Patch

Attachment 205843 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1022028

New failing tests:
fast/js/global-constructors-attributes-shared-worker.html
fast/forms/select/popup-closes-on-blur.html
fast/media/mq-transform-03.html
editing/selection/user-select-all-image-with-single-click.html
fast/forms/search-event-delay.html
fast/js/global-constructors-attributes-dedicated-worker.html
editing/selection/user-select-all-with-single-click.html
fast/media/mq-transform-02.html
platform/win/accessibility/multiple-select-element-role.html

[Build Bot]

Created attachment 205947 [details]
Archive of layout-test-results from APPLE-EWS-2 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-2  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Dean Jackson]

Comment on attachment 205843 [details]
Patch

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

> Source/WebCore/html/HTMLSelectElement.cpp:1351
> +        // This click or drag event was not over any of the options

Nit. End the comment with a full-stop/period.

[Ruth Fong]

Created attachment 205956 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 205956 [details]
Patch

Clearing flags on attachment: 205956

Committed r152330: <http://trac.webkit.org/changeset/152330>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

*** Bug 117431 has been marked as a duplicate of this bug. ***