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)

x86
All
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: gekacheka, Assigned: roc)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files, 1 obsolete file)

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
Attached file Example testcase (deleted) —
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: form → roc+moz
Keywords: regression, testcase
OS: Windows 2000 → All
Thanks Andrew. How serious is this? Likely to break any known sites?
Priority: -- → P3
> 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.
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.
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.
Attached patch UNTESTED patch (obsolete) (deleted) — Splinter Review
Here's what I'm trying to do. I'll test it tonight.
Blocks: 212753
Attached patch Simpler patch (deleted) — Splinter Review
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
Attachment #129516 - Flags: superreview?(dbaron)
Attachment #129516 - Flags: review?(dbaron)
Attachment #129516 - Flags: superreview?(dbaron)
Attachment #129516 - Flags: superreview+
Attachment #129516 - Flags: review?(dbaron)
Attachment #129516 - Flags: review+
Comment on attachment 129516 [details] [diff] [review] Simpler patch extremely low-risk patch for a regression
Attachment #129516 - Flags: approval1.5b?
I suggest drivers approve this for 1.5b, this would help web application developers :)
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+
Blocks: 215928
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Is there a bug on making events not bubble up out of the scrollbar (and is that really the right thing to do?)
Bug 215928. I think it is the right thing to do, but I'm not 100% sure.
*** Bug 227558 has been marked as a duplicate of this bug. ***
Depends on: 310647
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: