Buttons don't always react to mouse clicks

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
14 years ago
3 months ago

People

(Reporter: martijn.martijn, Assigned: roc)

Tracking

(Depends on 1 bug, Blocks 1 bug, {regression, testcase})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

14 years ago
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.
Reporter

Comment 1

14 years ago
Posted file testcase

Updated

14 years ago
OS: Windows XP → All
Hardware: PC → All

Comment 2

14 years ago
Related to bug 324396?

Comment 3

14 years ago
Probably not, since that regression was introduced at a different time.
Posted 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
Last Resolved: 13 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
Reporter

Updated

13 years ago
Depends on: 331590
Reporter

Comment 12

12 years ago
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
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.