15112 - when clicking a "disabled" button, the on click handler is still called.

Status: VERIFIED DUPLICATE of bug 14836

(Core :: XUL, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

I have a titled button, and I set the disabled attribute to "true"

when I click on it, the on click handler is still called.

alecf is coming up to speed on XUL, and has offered to fix this.

Comment 1

[Alec Flett]

I think I have the fix. It turns out we were not setting aEventStatus inside
nsTitledButton::HandleEvent.
testing my fix now

Comment 2

[Alec Flett]

nope, that wasn't it... the event is still coming through. It's possible that
the event is ignored by this frame but is being passed up to the document or
higher up in the frame tree.
I'm going to try adding a DOM listener to the content node, and see if i can
intercept the event there, then I can at least see the path it took.

Eric, I'm trying to get up to speed on XUL, etc... I figured this would be a
good place to start. Do you have any suggestions/pointers beyond what I've tried
thus far?

I would have thought that an even first arrived at a frame, and then got
propagated to the content.... do you think there are other frames, besides the
TitledButtonFrame that could be generating this event?

Comment 3

[Alec Flett]

woah, I found the problem! It was in the event manager.
While it's true we should be setting status to nsEventStatus_eConsumeNoDefault,
the event state manager was ignoring the status for mouseups.

Adding joki to the CC for approval/recommendation.

In nsEventStateManager::PostHandleEvent, we check if the status of the event is
nsEventStatus_eConsumeNoDefault in EVERY event EXCEPT the case of
NS_MOUSE_xxx_BUTTON_UP....

This seems bad, but I'm wondering if it was done for a reason...
If this was simply an oversight, I'd like to just add
      if (nsEventStatus_eConsumeNoDefault != aStatus) ....
around the whole statement.

at the very least we should use this check to determine whether or not to call
CheckForAndDispatchClick (which just dispatches the event to the frame and the
content node)

The only other thing that happens in PostHandleEvent on a mouseup is setting the
targetframe's mousedown state to PR_FALSE, so we may not want to block that one.
I don't know.

joki? I'll fix this if you just tell me the right thing to do:
- block CheckForAndDispatchClick AND the frameSel->SetMouseDownStatE(PR_FALSE)
- just the CheckForAndDispatchClick.

Comment 4

[cpratt]

Dupe of bug 14836?

Comment 5

[Phil Peterson]

Since 14836 is marked fixed, and I don't see this in the current build, I'm
going to mark a dup. Seth, could you verify that your code works now?


*** This bug has been marked as a duplicate of 14836 ***

Comment 6

[Alex]

Verifying as duplicate of 14836

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44cccc5700a2
[fenix] For https://github.com/mozilla-mobile/fenix/issues/15112 - Still show recently closed when history empty