Closed Bug 326827 Opened 17 years ago Closed 17 years ago

Buttons don't always react to mouse clicks

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: roc)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(2 files)

See upcoming testcase.

To reproduce:
- Mousedown on the text in the button
- Move away with the mouse into an 'empty' area of the button
- Release the mouse button, 

Actual result:
An alert appears

Expected result:
An alert with the text 'clicked' appears

This regressed between 2006-01-25 and 2006-01-26, so likely a regression from bug 317375.

I think this is the phenomenon I see that sometimes clicking on things in the browser doesn't work.
Attached file testcase
Blocks: 326732
OS: Windows XP → All
Hardware: PC → All
Related to bug 324396?
Probably not, since that regression was introduced at a different time.
Attached patch fixSplinter Review
Very simple fix. The button should not allow mouse events to target its children. (This is what the old GetFrameForPoint code did, I'm just copying that.)
Assignee: events → roc
Status: NEW → ASSIGNED
Attachment #211965 - Flags: superreview?(dbaron)
Attachment #211965 - Flags: review?(dbaron)
Note that in general that may be the wrong behavior; once we fix event propagation, we want an <a> inside <button> to trigger when clicked, no?
Can you specify exactly what the right behaviour should be? If I click on an <a>, should it trigger the button as well? If I click on a <div onclick="">, should that and/or the button trigger?

Of course we'll fix this regression before worrying about that in detail...
Comment on attachment 211965 [details] [diff] [review]
fix

r+sr=dbaron.

In general I think there were a lot of hacks in GetFrameForPoint that didn't belong there, especially in XUL code.  Should we have a bug on auditing for such things after the event changes?
Attachment #211965 - Flags: superreview?(dbaron)
Attachment #211965 - Flags: superreview+
Attachment #211965 - Flags: review?(dbaron)
Attachment #211965 - Flags: review+
(In reply to comment #7)
> (From update of attachment 211965 [details] [diff] [review] [edit])
> r+sr=dbaron.
> 
> In general I think there were a lot of hacks in GetFrameForPoint that didn't
> belong there, especially in XUL code.  Should we have a bug on auditing for
> such things after the event changes?

Sure. It's really easy to audit them now, each hack was turned into an nsDisplayItem subclass :-). (Except for 'allowevents', which moved into nsDisplayList::HitTest.)

Okay, I exaggerate slightly. You also want to look for IsForEventDelivery to see where different code paths are taken for event handling.
checked in
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Verified FIXED using build 2006-02-21-09 of SeaMonkey trunk on Windows XP on testcase: https://bugzilla.mozilla.org/attachment.cgi?id=211500&action=view
Status: RESOLVED → VERIFIED
Depends on: 331590
Backing this fix out fixes bug 183113.
I guess this really would be better fixed by a fix for bug 324396.
Blocks: 183113
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.