Closed
Bug 207637
Opened 21 years ago
Closed 21 years ago
Regression: Clicking in scrollbar of multiline <select> erroneously fires onchange
Categories
(Core :: Layout: Form Controls, defect, P2)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gekacheka, Assigned: roc)
References
Details
(Keywords: regression, testcase)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
asa
:
approval1.5b+
|
Details | Diff | Splinter Review |
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 2•21 years ago
|
||
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
Assignee | ||
Comment 3•21 years ago
|
||
Thanks Andrew.
How serious is this? Likely to break any known sites?
Priority: -- → P3
Comment 4•21 years ago
|
||
> 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•21 years ago
|
||
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.
Assignee | ||
Updated•21 years ago
|
Priority: P3 → P2
Assignee | ||
Comment 6•21 years ago
|
||
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.
Assignee | ||
Comment 7•21 years ago
|
||
Here's what I'm trying to do. I'll test it tonight.
Assignee | ||
Comment 8•21 years ago
|
||
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).
Attachment #129376 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #129516 -
Flags: superreview?(dbaron)
Attachment #129516 -
Flags: review?(dbaron)
Updated•21 years ago
|
Attachment #129516 -
Flags: superreview?(dbaron)
Attachment #129516 -
Flags: superreview+
Attachment #129516 -
Flags: review?(dbaron)
Attachment #129516 -
Flags: review+
Assignee | ||
Comment 9•21 years ago
|
||
Comment on attachment 129516 [details] [diff] [review]
Simpler patch
extremely low-risk patch for a regression
Attachment #129516 -
Flags: approval1.5b?
Comment 10•21 years ago
|
||
I suggest drivers approve this for 1.5b, this would help web application
developers :)
Comment 11•21 years ago
|
||
Comment on attachment 129516 [details] [diff] [review]
Simpler patch
a=asa (on behalf of drivers) for checkin to Mozilla 1.5beta.
Attachment #129516 -
Flags: approval1.5b? → approval1.5b+
Assignee | ||
Comment 12•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 13•21 years ago
|
||
Is there a bug on making events not bubble up out of the scrollbar (and is that
really the right thing to do?)
Assignee | ||
Comment 14•21 years ago
|
||
Bug 215928. I think it is the right thing to do, but I'm not 100% sure.
Comment 15•21 years ago
|
||
*** Bug 227558 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•