Buttons don't always react to mouse clicks

VERIFIED FIXED

Status

()

Core
Event Handling
VERIFIED FIXED
13 years ago
10 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: roc)

Tracking

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

Trunk
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

13 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

13 years ago
Created attachment 211500 [details]
testcase

Updated

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

Comment 2

13 years ago
Related to bug 324396?

Comment 3

13 years ago
Probably not, since that regression was introduced at a different time.
Created attachment 211965 [details] [diff] [review]
fix

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

11 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
You need to log in before you can comment on or make changes to this bug.