107048 - Move event specific properties from nsEvent into the specific event classes

Status: NEW

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

Description

[Garrett Smith]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:0.9.5) Gecko/20011015
BuildID:    2001101516

using a for in loop to iterate through a domAttrModifed event. Used to work in
Moz 0.9.2, but doesn't work anymore

Copied the error from the terminal.
Error: [Exception... "Illegal operation on WrappedNative prototype object" 
nsresult: "0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)"  location: "JS frame ::
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
:: getProps :: line 4"  data: no]
Source File:
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
Line: 4

Reproducible: Always
Steps to Reproduce:
1. read and then follow the steps of the demo


Actual Results:  Error: [Exception... "Illegal operation on WrappedNative
prototype object"  nsresult: "0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)" 
location: "JS frame ::
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
:: getProps :: line 4"  data: no]
Source File:
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
Line: 4

Expected Results:  The properties of the event should be exposed.

Here is the code that Moz has a problem with:

showProps is the main function (see bottom).

function getProps(event){
	var i=0, props="<table cellpadding='1'  cellspacing='1' >", ei;
	for(i in event){
		ei= event[i];
		if(typeof event[i] == "object") {
			ei= "<span style='font-weight: 800;color: #00f'>"+event[i]+"</span>";
		}
		else ei= "<span style='font-weight: 800;'>"+event[i]+"</span>";
		props+= "<tr><td>"
		+event 
		+"."
		+i
		+" = </td><td>"
		+ei
		+"</td></tr>";
	}
	return props+"</table>";
}
function showProps(event){ 
 var main= document.getElementById("main");
 	 var newDiv= main.appendChild(document.createElement("div"));
 	 newDiv.className= "list"
 	 newDiv.innerHTML= getProps(event);
 	 
}

Comment 1

[Greg K.]

Reporter, can you produce a minimal testcase for this?

Comment 2

[Vladimir Ermakov]

Attachment: testcase, document.body instead of main

Comment 3

[Vladimir Ermakov]

There is only one problem with the code, main is not a valid node in the tree..
if you use document.body as I did in the testcase it works fine.
To test click on the text in the testcase...
What build are you using? It works for me on windows 98 2001-10-23-09-trunk

Comment 4

[Garrett Smith]

Attachment: document.body, but with domAttrModified instead of document.onclick

Comment 5

[Garrett Smith]

The test case does not use a domAttrModified event listener.

When I used domAttrModified, the browser crashes.

Comment 6

[Garrett Smith]

Attachment: simplified test case -works without loop, crashes with loop.

Comment 7

[Vladimir Ermakov]

I believe domAttrModified crashes because of recursion, which is a known issue
in bug 77271... 

Comment 8

[Garrett Smith]

Attachment: try - catch : output errors or alert( properties )

Comment 9

[Vladimir Ermakov]

So, do you see the bug still, for me the alert shows the properties....

Comment 10

[Garrett Smith]

Ah, 

The properties are displayed in the alert,

but they cannot be displayed in the textarea (does not finish execution).

Try changing the code in the source:

function showProps(e){
    //var df0= document.forms[0];

    // if the showProperteis radio button is checked, 
    // in alert (takes forever).
    
    //if(df0["show"][0].checked) alert(examineObjectProperties(e)); 
    //else 
    document.getElementById("traceout").value= examineObjectProperties(e);
}

This'll burn my processor. I have to force quit Moz.
Back to the original report, I found that this file would not work in 0.9.5, but
it does work in 0.9.2:

http://dhtmlkitchen.com/dhtml/events/eventListener/load.html

It has to do with a property (prototype) in the domAttrModified event (guess).

Comment 11

[Vladimir Ermakov]

Attachment: testcase that filters out recursion

Comment 12

[Vladimir Ermakov]

Again the hang/crash you are seeing is bug bug 77271
 and the reason it is recursing is bug 97058 which fires domAttrModified event
when text area is modified...

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

Comment 13

[Garrett Smith]

Vladamir, 

This is NOT recursive! Read the code carefully. 

This is wrong:
*** This bug has been marked as a duplicate of 77271 ***



Recursion is when a function or method repeatedly calls upon itsself until a
condition has been met.

I use a control structure, a for(prop in obj) loop, NOT recursion.

Again, read the code before jumping to conclusions.

Garrett

Comment 14

[Vladimir Ermakov]

Here is what happens:

1) the attr of body is modified - attrModified event handler is called
-> The handler writes the attributes of the event into text area
2) The text area attribute is modified due to bug 97058 and handler is called
-> The handler writes the attributes of the event into text area
3) The text area attribute is modified due to bug 97058 and handler is called
-> The handler writes the attributes of the event into text area
4) The text area attribute is modified due to bug 97058 and handler is called
.... so on 100 more times or so
--- CRASH ---
Thats what I ment by recursive event :) 

Comment 15

[Garrett Smith]

Yeah, Vlad, I didn't see that. This is why:

* "value" is a property of a textarea element.

* "value" is an attribute of the text input element (and other input types).
* "value" is not an attribute of a textarea element.

You did a good job with finding that loop. (which has nothing to do with the
original problem.)

I just wanted to add that there are some problems with domAttrModified

1 (as discussed in another bug) --prevValue is empty string
2 Several Errors while accessing certain props (by checking the 'show errors' radio.

On No. 2, the problem could be prevented by not allowing all properties from the
event object to be inherited, or at least giving them a value of "undefined"
--not throwing an error.


My view:
A textarea change should fire a different type of event.
Technically speaking, the value of a textarea is a text node, the nodeValue of
the the textarea's firstChild.

I don't know what the best way to address this, but calling "value" an attribute
of textarea is not the way. There's a lot of other events
(domNodeInsertedIntoDocument, et c.) ...I ought to review the w3c spec more.

Comment 16

[Vladimir Ermakov]

So Garrett, the original issue reported here works for me. I demonstrated in the
attachment with the counter to work around recursion of events that the
properties of the domAttrModified event are exposable. Can I resolve this bug as
works for me?

Comment 17

[Garrett Smith]

There are really 3 bugs here. 


1- Unless a try-catch is used, this error will be produced:

Error: [Exception... "Illegal operation on WrappedNative
prototype object"  nsresult: "0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)" 
location: "JS frame ::
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
:: getProps :: line 4"  data: no]
Source File:
file:///OS%20X/Users/garretts/Sites/DHTMLkitchen/dhtml/events/eventListener/showEvent.js
Line: 4

 --the original problem I reported

2- Recursion exposed another bug -value treated as an attribute of textarea (it
isn't).


3- unstopped recursion --Vladimir, you brought our attention to this.

Assign them as you see fit, or guide me to the source. With a little guidance, I
might be able to work on them. 

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attaching a testcase that avoids the looping problems (those are covered by bug
97058 and bug 77271 as [e-mail] pointed out).

The problem seems to be the following.  The MutationEvent produced has a bunch
of properties ("screenX", "button", "view", etc) that just do not apply to
mutation events (they apply to mouse events, or UI events, or the like).  Trying
to access those properties throws the exception Garrett noted.

I'm seeing this on Linux, so OS to ALL.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that demonstrates the problem clearly

Comment 20

[Daniel Wang]

FYI, typeof(object[blah]) seems only to throw exception when blah
is a XUL-specific property. This happens to all objects (I could
reproduce this with document object, for example)

Comment 21

[Daniel Wang]

just simple object[xul_property] (w/o typeof) also throws exception.

not sure if this is by design, but could be useful to extract an array
of XUL-only properties :p

Comment 22

[Vladimir Ermakov]

Hmm. They do have a class of their own, but all that stuff is inherited from
nsEvent. Its not only Mutation events that show properties of other event types,
MouseEvents for example also show keyevent, and mutation event properties. I
dont know enough about the architecture to make a judgement whether it would be
a good idea to move these properties or not. 

Comment 23

[Vladimir Ermakov]

Changing description. Could potentially lower memory usage, but could also break
a lot of dependent code.

Comment 24

[Garrett Smith]

MouseEvent should have those props.

citation:
Object MouseEvent

    MouseEvent has the all the properties and methods of the UIEvent object as
well as the properties and methods defined below.
    The MouseEvent object has the following properties:

        screenX
            This read-only property is a long object.
        screenY
            This read-only property is a long object.
        clientX
            This read-only property is a long object.
        clientY
            This read-only property is a long object.
        ctrlKey
            This read-only property is of type Boolean.
        shiftKey
            This read-only property is of type Boolean.
        altKey
            This read-only property is of type Boolean.
        metaKey
            This read-only property is of type Boolean.
        button
            This read-only property is of type Number.
        relatedTarget
            This read-only property is a EventTarget object.

http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/ecma-script-binding.html

Mutation Event, however, does not. The heirarchy may be off, I guess. It looks
like there's too much stuff in the top level interface, Event.

Comment 25

[Vladimir Ermakov]

I dont think keyCode, DOM_VK_** belong to mouse events..

Comment 26

[Vedran Miletic]

retargeting