179974 - cant set a _working_ onkeypress on input-element with javascript

Status: RESOLVED DUPLICATE of bug 54035

(Core :: DOM: Events, defect, P3)

Description

[Mario Ivankovits]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4 AND Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021113

Problem when set event-function "onkeypressfunc()" via javascript.
The function is called, but the supress of a key-press is not possible.
Attached is a html-file which makes it possible to reproduce the problem.

Reproducible: Always

Steps to Reproduce:



Expected Results:  
supress the keypress

Comment 1

[Mario Ivankovits]

Attachment: test case for working and not working onkeypress handler

Comment 2

[Boris Zbarsky [:bzbarsky]]

Try setting the listeners based on a button click or something after the doc has
loaded?  If that helps, this is bug 99820.

Comment 3

[Mario Ivankovits]

Attachment: onkeypress event handler initialized on button click

initialize the onkeypress event handler on button click after page has loaded
does not change anything.
The eventhandler is called but the supression of the keypress wont work anyway.

Comment 4

[Mario Ivankovits]

Maybe this is nonsense, but:
I have installed the sources of mozilla and tried some debugging, which was not
easy.

What i have found is (not much):
There is more than one event-listener attached to one html-element, i think
these are some internal listener (maybe needet to dispatch the keypress to the
native widget ?)
During "nsEventListenerManager::HandleEvent" in NS_KEY_* all these
event-listeners are notified.

In case of adding an onkeypress event listener in the html source the
eventlistener is on the very beginning, and so it is possible to cath the event.

But the onkeypress event listener added using javascript is the last in the
event-listener list, and so the keypress is already dispatched to the native widget.

However, for me there seems to be a second problem:
During enumerating the key*-listeners in "nsEventListenerManager::HandleEvent"
the "return value" (ret) is only honored from the last Listener. The
"aEventStatus = nsEventStatus_eConsumeNoDefault" is set on exit. So if some
other listener would like to cancel the event it has to set this status
correctly. This is not consistent, sometimes "return false" would work and
sometimes it wont.

Please ignore this comment, if i have completley misunderstood the internals.

Comment 5

[Mario Ivankovits]

I have found, that my last comment is definitley true. The last Listener in the
key-event-listener list is the nsTextEditorKeyListener. A listener added to an
input-field by javascript is places AFTER the nsTextEditorKeyListener, but then
the keypress is already processed.

I have patched "nsEventListenerManager::AddEventListener" like this

*** OLD-CODE Starting at line 484:
  if (!found) {
    ls = PR_NEW(nsListenerStruct);
    if (ls) {
      ls->mListener = aListener;
      ls->mFlags = aFlags;
      ls->mSubType = aSubType;
      ls->mSubTypeCapture = NS_EVENT_BITS_NONE;
      ls->mHandlerIsString = 0;
      ls->mGroupFlags = group;
      listeners->AppendElement((void*)ls);
      NS_ADDREF(aListener);
    }


*** changed to ***

  if (!found) {
    ls = PR_NEW(nsListenerStruct);
    if (ls) {
      ls->mListener = aListener;
      ls->mFlags = aFlags;
      ls->mSubType = aSubType;
      ls->mSubTypeCapture = NS_EVENT_BITS_NONE;
      ls->mHandlerIsString = 0;
      ls->mGroupFlags = group;

+      PRBool isJS = PR_FALSE;
+      if (aFlags & NS_PRIV_EVENT_FLAG_SCRIPT)
+      {
+            isJS = PR_TRUE;
+      }
+//      else
+//      {
+//            nsCOMPtr<nsXPCWrappedJSClass> isWrappedJSEvt =
do_QueryInterface(aListener);
+//            if (isWrappedJSEvt)
+//            {
+//                  isJS = PR_TRUE;
+//            }
+//      }
+      if (listeners->Count() > 0 && isJS)
+      {
+            listeners->InsertElementAt((void*) ls, 0);
+      }
+      else
+      {
-       listeners->AppendElement((void*)ls);
+            listeners->AppendElement((void*)ls);
+      }
      NS_ADDREF(aListener);
    }


If there is an Script-Event-Handler (JavaScript) which sould be inserted into
the Event-Handler-List, and there are already Event-Handler in the list, then
the new event-handler is inserted at the top of the list.
Now Field 3 works great with this patch.

Field 4 fails anyway. When adding an eventHandler with "addEventListener" in
JavaScript. The JavaScript object is wrapped in an nsXPC* Object.
The check in my source-comment has to be more sophisticated.

Any hint how to solve this?

Comment 6

[Mario Ivankovits]

Comment#2 does not help, so this might not be bug 99820.
I have removed the dependency to this bug.

Comment 7

[Mario Ivankovits]

Attachment: Patch made onkey* events working correctly when added in javascript

This patch changes how nsEventListenerManager::AddEventListener inserts new
event-listener.
If a new event-listener is an java-script and there are already more than 2
listeners, then the new listener will be inserted before the the last listener.
The last listener dispatch the event to the widget.

Note: one has to call "event.preventDefault()" within the event-handler.
If a "return false" should be sufficient, then the
"nsEventListenerManager::HandleEvent" has to be fixed also.

Comment 8

[Mario Ivankovits]

Attachment: test case for working and not working onkeypress handler (added preventDefault())

Comment 9

[Mario Ivankovits]

Attachment: onkeypress event handler initialized on button click (added preventDefault())

Comment 10

[saari (gone)]

->bryner

Comment 11

[Asa Dotzler [:asa]]

Mario, please do not set flags if you don't know how to use them. See
http://bugzilla.mozilla.org/flag-help.html for information on how they work. You
make a request with the ? and it is plussed or minused by someone else. You do
not set the plus yourself. For this particular flag the only people that can set
"+" are [e-mail]. Also, only nominate bugs using this flag if you
believe that they should block the release of 1.3a.

Comment 12

[Mario Ivankovits]

@asa: Sorry for this, but the document does not talk about a restriction about 
setting flags. While setting the flag for patches to "?" was clear for me, 
setting the "?" for the "blocking1.3a" wasnt. Sorry again.

However i will nominate this bug as an "blocking1.3a" bug now. (I hope this is 
done correctly now)
I think there are web-applications out there (like ours), where one use java-
script to attach the onkey* event-handlers to allow input-checks.
e.g. We use this technique to avoid entering alpha-chars in numeric fields.

Comment 13

[Asa Dotzler [:asa]]

not going to hold 1.3a for this.

Comment 14

[John Keiser (jkeiser)]

Comment on attachment 106929 [details] [diff] [review]
Patch made onkey* events working correctly when added in javascript

This looks very very much like a hack.	If I had to guess the reason behind the
patch, I would guess that there are two system event handlers of this type that
happen to need to be run last.	Because onxxx probably runs before these get
attached, no problem exists, but the JS runs after that.  This is a very
fragile way to handle this situation.

Do I have the analysis right?  It seems you have analyzed the cause.

The right way to do this is to have the system handlers run in the system event
phase--perhaps this bug is already fixed anyway ...

Bryner?

Comment 15

[John Keiser (jkeiser)]

This bug is in fact not fixed.  The proper way to fix it would be to place the
system event listeners in a real working system event group.  Barring that, a
possibly acceptable way to fix it would be to walk the list of listeners and
place the JS handler before any system event handler (and put any system event
handler after JS handlers), if there is a bit or something that tells us this
information.

Comment 16

[Vladimir Ermakov]

*** Bug 163435 has been marked as a duplicate of this bug. ***

Comment 17

[Vladimir Ermakov]

*** Bug 188421 has been marked as a duplicate of this bug. ***

Comment 18

[Mario Ivankovits]

I dont have followed the mozilla development in this detail. Is there now a 
chance to know if a event handler is an "system" event handler and to implement 
a logic as described in the second part of comment#15?

I think at least we have to know if the event handler is the one which dispatch 
the event to the widget.

Comment 19

[Brian Ryner (not reading)]

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