207637 - Regression: Clicking in scrollbar of multiline <select> erroneously fires onchange

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030529
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030529

In a multiline <select>, once a selection is clicked, any click in the
<select>'s scrollbar erroneously fires the onchange=... handler.

Regression: the problem does not occur in Mozilla 1.3

Onchange should not be fired when just scrolling or clicking in the scrollbar.



Reproducible: Always

Steps to Reproduce:
Load example.

0. Click in the scrollbar in several places.<p>

a. When first loaded, clicking in the scrollbar does not fire
onchange.  This is correct.<p>

(b. When first loaded, clicking anywhere in the scroll bar region scrolls
one pane's worth.)<p>

1. Change the selected item (click an item in list).<p>

The onchange handler is fired.  This is correct.<p>

2. Click in scrollbar.<p>

a. Now clicking in scroll bar fires onchange EVERY time.
This is INCORRECT, scrolling/clicking in scrollbar should
NEVER fire onchange.<p>

(b. Now clicking in the scrollbar region may scroll more than one
pane's worth, as if repeatedly clicked until scrolled to point of
click.  This is inconsistent with 0.b.)<p>

Actual Results:  
After step 1, any scroll/click in the scroll bar fires onchange, popping up the
onchange alert.

Expected Results:  
At any time, scrolling/clicking in the scroll bar should never fire onchange,
never pop up the onchange alert.

HTML 4.01 says:

onchange = script [CT]
    The onchange event occurs when a control loses the input focus and its value
has been modified since gaining focus. This attribute applies to the following
elements: INPUT, SELECT, and TEXTAREA.
http://www.w3.org/TR/REC-html40/interact/scripts.html#adef-onchange

Comment 1

[gekacheka]

Attachment: Example testcase

Comment 2

[Andrew Schultz]

I'm seeing this with linux trunk 20030609
this regressed between linux turnk 2003040805 and 2003040905, indicating the
gfx-scroll-fun (bug 126263 or perhaps bug 201300) is responsible for this change.

==> roc

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks Andrew.

How serious is this? Likely to break any known sites?

Comment 4

[Andrew Schultz]

> How serious is this? Likely to break any known sites?

I found this bug after trying to use http://bugzilla.mozdev.org/query.cgi (older
version of Bugzilla with a perf problem selecting the "Program").  this bug
makes that form virtually unusable.  I would guess that type of performance
problem exaggeration is probably the most noticable effect of this bug.  I doubt
it would actually lead to the "wrong" behavior.

Comment 5

[Doron Rosenberg (IBM)]

This is kinda annoying, and is used a lot in web applications.  Though it
probably doesn't break much, its a performance issue for such applications.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Looks like the same issue is causing a worse bug in 212753.

The problem is that the DOM event listener on the listbox is seeing scrollbar
mouse events as they bubble out of the anonymous scrollbar.

One thing that should be fixed is that nsListControlFrame::MouseUp should set
mChangesSinceDragStart to PR_FALSE, so that mouse-ups without prior mouse-downs
don't just see the old value of mChangesSinceDragStart. (Right now, after the
selection has been changed due to a click, mChangesSinceDragStart remains
PR_TRUE so future MouseUps just see that and fire an onchange.)

The real fix is to whack scrollbars so that they prevent bubbling of their mouse
events. I was working on how to do this but my Mozilla dev. machine went AWOL. I
will work on it tonight.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: UNTESTED patch

Here's what I'm trying to do. I'll test it tonight.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Simpler patch

Only the nsListControlFrame.cpp part of the patch is required to fix this bug.

It may be desirable to make scrollbars not bubble their events, but that
doesn't have to be done here (and maybe should be done in some other way).

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 129516 [details] [diff] [review]
Simpler patch

extremely low-risk patch for a regression

Comment 10

[Doron Rosenberg (IBM)]

I suggest drivers approve this for 1.5b, this would help web application
developers :)

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 129516 [details] [diff] [review]
Simpler patch

a=asa (on behalf of drivers) for checkin to Mozilla 1.5beta.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fix checked in.

Comment 13

[jag (Peter Annema)]

Is there a bug on making events not bubble up out of the scrollbar (and is that
really the right thing to do?)

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Bug 215928. I think it is the right thing to do, but I'm not 100% sure.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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