The default bug view has changed. See this FAQ.

Do not prevent event dispatching even if there is no prescontext or (form) element is disabled

NEW
Unassigned

Status

()

Core
DOM: Events
11 years ago
10 days ago

People

(Reporter: smaug, Unassigned)

Tracking

(Blocks: 3 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Do not prevent event dispatching even if there is no prescontext or (form) element is disabled. Currently for example mutation events don't work if form elements are
disabled.
(Reporter)

Updated

11 years ago
Blocks: 274626

Comment 1

11 years ago
How would you prefer to handle it?
a) change the early return NS_OK; to return super::PreHandleEvent(aVisitor);
b) change the sense of the conditions and indent all the code
c) extract the control-specific code into a separate function (named?)
(Reporter)

Comment 2

11 years ago
Would it be enough to move the 
(disabled || !aVisitor || uiStyle->mUserInput == NS_STYLE_USER_INPUT_NONE ||
 uiStyle->mUserInput == NS_STYLE_USER_INPUT_DISABLED)
checks to PostHandleEvent and do default handling only if that
expression evaluates to false.

Comment 3

11 years ago
Sadly form controls have to do some prehandling too. For instance, checkboxes need to update their value before the event propagates, but then need to reset their value if the event is subsequently cancelled.
(Reporter)

Comment 4

11 years ago
(In reply to comment #3)
> Sadly form controls have to do some prehandling too. For instance, checkboxes
> need to update their value before the event propagates, but then need to reset
> their value if the event is subsequently cancelled.
> 
Of course. :(
Well, then a) but perhaps adding flag to mItemFlags to indicate
that posthandling should not be done?

Comment 5

11 years ago
Is there a way to indicate that only chrome event handlers should be called?
(Reporter)

Comment 6

11 years ago
(In reply to comment #5)
> Is there a way to indicate that only chrome event handlers should be called?
> 

What is a "chrome event handler"? Atm there is no way to detect that an event
listener was added in chrome. We could perhaps (at least in some cases) use
'trusted' flag for that, that is set by default when registering listeners in chrome.

Comment 7

11 years ago
(In reply to comment #6)
>(In reply to comment #5)
>>Is there a way to indicate that only chrome event handlers should be called?
>What is a "chrome event handler"?
That's a good question. I was going to say it was an event handler on a event target in an object managed by a docshell with chrome type. However your idea of firing only trusted event handlers is equally valid.

For people not following the discussion on IRC the issue is that content does not expect to see events fired for disabled form controls. However this then causes bugs because our browser chrome (e.g. tooltips) does want to see them.
(Reporter)

Comment 8

11 years ago
Some events should be fired, I think. mousemove, mouseover, mouseout? but not click, keypress?
Jonas, I sent a mail to webapi list, but didn't get any comments.
Could you raise the issue on a conf call?
I'm not really sure what we're doing with HTML specific event issues, i've raised that issue with the WG.
If our only concern is chrome vs content, don't we have the NO_CONTENT_DISPATCH flag for that?

Note that I agree that we should fire mousemove....  click is a weird one; what do other browsers do?

Comment 11

11 years ago
(In reply to comment #10)
>click is a weird one; what do other browsers do?
IE seems not to generate a click event when you click on a disabled input.
(Reporter)

Comment 12

11 years ago
Created attachment 234919 [details] [diff] [review]
fixes mutation events and explicit event dispatching
Assignee: events → Olli.Pettay
Status: NEW → ASSIGNED
(Reporter)

Comment 13

11 years ago
Comment on attachment 234919 [details] [diff] [review]
fixes mutation events and explicit event dispatching

I think I want this :)
Makes DOM Events (when dispatched using dispatchEvent) to work always.
Also mutation events work and I think it is quite clear when an event is "presentation event", at least I hope so.
Attachment #234919 - Flags: review?(jst)
Comment on attachment 234919 [details] [diff] [review]
fixes mutation events and explicit event dispatching

- In content/events/public/nsEventDispatcher.h:

   /**
+   * mPresentationDispatch is PR_TRUE whenever event is dispatched by native

Add "an" in front of "event".

r=jst
Attachment #234919 - Flags: review?(jst) → review+
(Reporter)

Comment 15

11 years ago
Comment on attachment 234919 [details] [diff] [review]
fixes mutation events and explicit event dispatching

Boris, what do you think about this?
Attachment #234919 - Flags: superreview?(bzbarsky)
I'm not going to have time for this review until after Sept 3.  :(
Comment on attachment 234919 [details] [diff] [review]
fixes mutation events and explicit event dispatching

>Index: content/events/public/nsEventDispatcher.h
>@@ -98,16 +100,25 @@ public:
>+   * mPresentationDispatch is PR_TRUE whenever event is dispatched by native
>+   * code which handles the UI and layout.

So it doesn't have to have anything to do with the presentation, then?  For example, should DOMContentLoaded events have mPresentationDispatch true or false?  What about DOMLinkAdded?  If they're dispatched via nsContentUtils::DispatchTrustedEvent they won't default to the same thing as if they're dispatched via Dispatch() or DispatchEvent()....

It feels like we need better docs here at least.

>   static nsresult Dispatch(nsISupports* aTarget,

Document what the new arg means?  For that matter, what does aCallback mean?  And aEventStatus?  How is aDOMEvent related to aEvent?  I feel like I asked some of these when I was reviewing this code....

Can we possibly make the new arg not optional?  It seems like the sort of thing where people will totally forget it's there, otherwise.

>   static nsresult DispatchDOMEvent(nsISupports* aTarget,


Same here, esp. because this function doesn't have any optional args to start with...  And again, document the arg.

I'd like to see a diff -w of content/html/content/src; that should be much more reviewable.
Attachment #234919 - Flags: superreview?(bzbarsky) → superreview-
(Reporter)

Updated

10 years ago
Summary: Do not prevent event dispatching even if there is no prescontext or (form) element is disabled → Do not prevent event dispatching even if (form) element is disabled
(Reporter)

Updated

10 years ago
Summary: Do not prevent event dispatching even if (form) element is disabled → Do not prevent event dispatching even if there is no prescontext or (form) element is disabled
(Reporter)

Comment 18

10 years ago
Created attachment 272948 [details]
some tests (synthetic event dispatch and also manually testable)

Opera doesn't prevent synthetic events when controls are disabled, Safari 
does seem to prevent (our current behavior which doesn't make sense). 
In Opera if click() is called on disabled control, click event is dispatched - not sure whether we want to do that.
Opera has some bug with disabled controls. After pressing some keys, disabled 
controls start to get keyup/down/press events.
(Reporter)

Comment 19

10 years ago
Created attachment 272949 [details] [diff] [review]
Disable event handling only when doing "full event dispatch" via ESM/Presshell

(Mutation event handling without prescontext is bug 388746.)
Attachment #234919 - Attachment is obsolete: true
Attachment #272949 - Flags: review?(jst)
(Reporter)

Comment 20

10 years ago
Created attachment 272950 [details] [diff] [review]
-w

I don't know how else we could easily recognize that the event is really coming
from the user, not a synthetic event dispatched by some script.
The first patch for this bug had pretty much the same idea, but the implementation
was much uglier.
(Reporter)

Updated

10 years ago
Attachment #272950 - Attachment is patch: true
Attachment #272950 - Attachment mime type: text/x-patch → text/plain
(Reporter)

Updated

9 years ago
Attachment #272949 - Flags: review?(jst)
QA Contact: ian → events
Blocks: 218093
Duplicate of this bug: 534510
Duplicate of this bug: 561784
(Reporter)

Updated

4 years ago
Duplicate of this bug: 867418

Updated

4 years ago
Duplicate of this bug: 880689
So, people still need the fix.
Blocks: 1067886
Duplicate of this bug: 1107929

Updated

2 years ago
Blocks: 541110

Updated

2 years ago
Blocks: 963529
Comment hidden (abusive)
Thomas, Olli does take his responsibilities seriously, as you can see by the number of bugs he fixes if you cared to look.  Unfortunately, he has more on his plate than anyone could possibly have time for, so he has to prioritize.

Please refrain from personal abuse in this bug database in the future; that sort of behavior is not acceptable here.

Comment 29

7 months ago
Boris, I fully respect the amount of work he, and everyone else for that matter, does - but it's pretty disappointing when I have my precious time wasted due to bugs that have been ASSIGNED for 10+ years to the same person without progress. Your own guidelines state that you should not mess with bugs assigned to other people, so maybe such bugs should then be unassigned and left for others to pick up? Instead of having one person just holding on to it for a decade, while everyone else ignores it?

That's my opinion, and it's not a personal assault - it's a comment on what appears to be a broken issue tracking strategy, for a browser that is causing me and other web developers ever increasing pain.
End of discussion.
(Reporter)

Comment 30

7 months ago
Feel free to take this bug.
And sorry that I've been busy with other stuff.
(I'll try to find time for this if no one else picks this up.)
Assignee: bugs → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(bugs)

Comment 31

10 days ago
Hello all,

This bug is really driving me crazy. Imagine the following situation:
- You have a selectbox field that's disabled (since readonly attribute doesn't work as per html spec), I have no other standard option.
- You need to have this value updated depending on what the user will do with the rest of the form's elements

Check.

Your only alternative is to try to emulate the disabled look n feel like onfocus=this.blur() along with a css.
The only problem is that this currently doesn't work (bug https://bugzilla.mozilla.org/show_bug.cgi?id=1334155).
There might also be some other work-around, but image that this has to apply to any other element too.
And again, you never can be sure that the work-around won't be broken with the next version.

So, checkmate.

Please correct me if I'm missing something.

Btw, chrome doesn't have problem dispatching events to disabled items. On the other hand the onfocus=this.blur() doesn't work at all and seems like they won't fix it.
You need to log in before you can comment on or make changes to this bug.