456151 - Cannot wrap DOM element prototype methods 'addEventListener' and 'removeEventListener'

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[ChristopherBalz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

Cannot wrap 'Element''s or 'HTMLDivElement''s 'addEventListener' method

This bug report includes working examples of the issue that you can run right now at a url.  Scroll down to "= = = Working Examples = = =" to skip to them.


= = = Overview = = = 

On Firefox (all versions), there seems to be no effective way to extend the 'addEventListener' method by modifying the prototypes of the constructors of html-element dom objects.  Because of this, object-oriented event listening becomes much more cumbersome than it needs to be. "Object oriented event listening" means to enable methods (as opposed to procedures (functions)) to listen to events.  

To get object-oriented event listening (in an elegant way, at least) in the current situation on Firefox, one's code must modify each respective dom object individually.  FF's JavaScript should be extensible enough to avoid this.  JavaScript frameworks rely on the consistently extensible nature of JavaScript.  But it seems broken here with the dom implementation of event listeners on Firefox.

Having to update each dom object is comparatively slow and is out of keeping with the ability to modify all the other properties of the constructor prototypes.  Fortunately, Firefox (esp. FF 3) is so fast that the performance hit is not giant.  

However, Safari 3 is not encumbered with this problem at all.  On Safari 3, for example, "A" below works fine.  But Firefox fails.  Safari 3 works because on that browser one may wrap the 'addEventListener' method of 'Element.prototype' and voila, all html dom elements see the change.

Firefox has 'addEventListener' methods for even low-level constructors such as 'HTMLDivElement.prototype'.  So perhaps those are the methods to override.  But, for example, when doing 

        'Element.prep( HTMLDivElement.prototype ) ' 

the wrap fails.  Previously, the Venkman debugger shows this error (same on FF 3 and 2):

     Exception ``[Exception... "Illegal operation on WrappedNative prototype object" nsresult: "0x8057000c" 

Currently, on FF 3, it shows

    Error ``this.emitEvent is not a function'' [x-] in file ``https://a248.e.akamai.net/n/248/1777/akssr20080909.0/www.etrade.com/javascript/et1/src/lib/model/messenger/request/mechanism/ScriptInclude.js'', line 125, character 0.
    Stopped for error handler.
    #0: function anonymous(p_eEvent=Event:{0}) in <https://a248.e.akamai.net/n/248/1777/akssr20080909.0/www.etrade.com/javascript/et1/src/lib/model/messenger/request/mechanism/ScriptInclude.js> line 125
    123: eventLoad : function( p_eEvent ) {
    124: // Let the application know that a response has been received from the server:
    125: this.emitEvent( p_eEvent );
    126: },
    127:

Then if one enters, from the stopped point in the code above:

    this

The result is: 

    $[0] = [HTMLScriptElement] [class: HTMLScriptElement] {1}

This is because wrapping of the 'addEventListener' method failed, and the scope of the event handler is now the DOM object, not the custom application-level JavaScript object intended.  


= = = Working Examples = = = 

  A) Test Case (shows Issue; do not see "Test Succeeded" alert):

     https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src-bug-demo.html

  B) With Workaround (Works; do see "Test Succeeded" alert):

     https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src.html

     
= = = Code Detail = = = 

   Compare lines 477 to 482 in 

      https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/js/bug_demo/Element.js 
 
   and in

      https://us.etrade.com/javascript/et1/src/core/dom/Element.js


= = = Summary = = =  

     A) Shouldn't at least the "Illegal operation on WrappedNative prototype object" be reported as a bug in the dom implementation?  

     B) Wouldn't the ideal behavior be that exhibited by Safari 3, where for example it is very easy to extend 'addEventListener'?  This would help to "supercharge" web applications.


   For more info see: http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/4b533df3ccef0c73/ea2f2227d484a4d1?lnk=raot


Reproducible: Always

Steps to Reproduce:
1. Go to https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src-bug-demo.html
2. Click 'OK' on "Proceed with test" alert.
3. View error in Error console or in Venkman JavaScript debugger.
Actual Results:  
Get this error: 

wrapping of the 'addEventListener' method failed, and the scope of the event is now the DOM object, not the custom application-level JavaScript object intended.  

Wrapping of the 'addEventListener' method failed, and the scope of the event handler is now the DOM object, not the custom application-level JavaScript object intended.  


Expected Results:  
Should see "Test Succeeded" alert dialog box.

View workaround at:

     https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src.html

Note that the workaround results in having to modify the instance of every DOM element that we would like to use the framework 'addEventListener' on.  Safari 3, by contrast, lets us modify just 'Element.prototype' and we are set for all elements.  That's an order of magnitude more efficient.

Comment 1

[ChristopherBalz]

Under 'Actual Results', the bug report should say: 

    Error ``this.emitEvent is not a function'' [x-] in file
``https://a248.e.akamai.net/n/248/1777/akssr20080909.0/www.etrade.com/javascript/et1/src/lib/model/messenger/request/mechanism/ScriptInclude.js'',
line 125, character 0.
    Stopped for error handler.

Comment 2

[Jesse Ruderman]

This might be a dup of bug 428229, but I can't tell because the reporter's testcase is gigantic.

Comment 3

[ChristopherBalz]

To address Jesse's concern, here is the core of the test case:

Lines 378 to 411 of JavaScript include 'Element.js' contain the code that directly operates on the 'addEventListener' method.  Note that some of the same operations are performed on 'removeEventListener.  You can easily view the file 'Element.js' by using the JSView add-on to Firefox.  

The "business ends" of this code are:

  A cache operation (comments explain the 'valueOf' call):

       oE.plainAddEventListener = oE.addEventListener.valueOf(); 
    } else {
       oE.plainAddEventListener = oE.addEventListener; 

and the wrap:

   oE.addEventListener = function( p_sEventName, p_fnHandler, p_oContext, p_bCapture ) {                 
      var fnHandler = Et1.getHandler( p_fnHandler, p_oContext );
      this.plainAddEventListener( p_sEventName, fnHandler, p_bCapture ); 

      return fnHandler;
   };

Hope that clears it up.

Comment 4

[Jesse Ruderman]

is oE an element, or is it something in each element's prototype chain, such as Node?  If it's a prototype, this is a dup of bug 428229.

Comment 5

[ChristopherBalz]

In the bug demo ( https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src-bug-demo.html ), oE is a prototype.  

In the working case (with the workaround), oE is an 'Element'.  The workaround is at: https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src.html

Detail: The workaround is to augment *every* concrete instantiated 'Element' that requires special framework functionality.   The ideal, much higher-performance way to go is to be able to augment the prototype instead.  As noted above, this works great on Safari 3/WebKit and would benefit Web applications greatly.

Comment 6

[ChristopherBalz]

Please note that this bug is not necessarily a duplicate of bug 428229, since this test case not only overrides 'addEventListener' but also *wraps* it.  In other words, the native functionality is preserved.  Also, this test case covers 'removeEventListener'.  Both need to work.

If marking this bug as a duplicate, you may wish to migrate the test case, as bug 428229 has no runnable test case.  Also, the test case on this bug demonstrates that a) there is a problem and b) the problem matters to users and creators of web applications that lots of people use.

Comment 8

[ChristopherBalz]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1)
> Gecko/2008070208 Firefox/3.0.1
> Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1)
> Gecko/2008070208 Firefox/3.0.1
> 
> Cannot wrap 'Element''s or 'HTMLDivElement''s 'addEventListener' method
> 
> This bug report includes working examples of the issue that you can run right
> now at a url.  Scroll down to "= = = Working Examples = = =" to skip to them.
> 
> 
> = = = Overview = = = 
> 
> On Firefox (all versions), there seems to be no effective way to extend the
> 'addEventListener' method by modifying the prototypes of the constructors of
> html-element dom objects.  Because of this, object-oriented event listening
> becomes much more cumbersome than it needs to be. "Object oriented event
> listening" means to enable methods (as opposed to procedures (functions)) to
> listen to events.  
> 
> To get object-oriented event listening (in an elegant way, at least) in the
> current situation on Firefox, one's code must modify each respective dom object
> individually.  FF's JavaScript should be extensible enough to avoid this. 
> JavaScript frameworks rely on the consistently extensible nature of JavaScript.
>  But it seems broken here with the dom implementation of event listeners on
> Firefox.
> 
> Having to update each dom object is comparatively slow and is out of keeping
> with the ability to modify all the other properties of the constructor
> prototypes.  Fortunately, Firefox (esp. FF 3) is so fast that the performance
> hit is not giant.  
> 
> However, Safari 3 is not encumbered with this problem at all.  On Safari 3, for
> example, "A" below works fine.  But Firefox fails.  Safari 3 works because on
> that browser one may wrap the 'addEventListener' method of 'Element.prototype'
> and voila, all html dom elements see the change.
> 
> Firefox has 'addEventListener' methods for even low-level constructors such as
> 'HTMLDivElement.prototype'.  So perhaps those are the methods to override. 
> But, for example, when doing 
> 
>         'Element.prep( HTMLDivElement.prototype ) ' 
> 
> the wrap fails.  Previously, the Venkman debugger shows this error (same on FF
> 3 and 2):
> 
>      Exception ``[Exception... "Illegal operation on WrappedNative prototype
> object" nsresult: "0x8057000c" 
> 
> Currently, on FF 3, it shows
> 
>     Error ``this.emitEvent is not a function'' [x-] in file
> ``https://a248.e.akamai.net/n/248/1777/akssr20080909.0/www.etrade.com/javascript/et1/src/lib/model/messenger/request/mechanism/ScriptInclude.js'',
> line 125, character 0.
>     Stopped for error handler.
>     #0: function anonymous(p_eEvent=Event:{0}) in
> <https://a248.e.akamai.net/n/248/1777/akssr20080909.0/www.etrade.com/javascript/et1/src/lib/model/messenger/request/mechanism/ScriptInclude.js>
> line 125
>     123: eventLoad : function( p_eEvent ) {
>     124: // Let the application know that a response has been received from the
> server:
>     125: this.emitEvent( p_eEvent );
>     126: },
>     127:
> 
> Then if one enters, from the stopped point in the code above:
> 
>     this
> 
> The result is: 
> 
>     $[0] = [HTMLScriptElement] [class: HTMLScriptElement] {1}
> 
> This is because wrapping of the 'addEventListener' method failed, and the scope
> of the event handler is now the DOM object, not the custom application-level
> JavaScript object intended.  
> 
> 
> = = = Working Examples = = = 
> 
>   A) Test Case (shows Issue; do not see "Test Succeeded" alert):
> 
>     
> https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src-bug-demo.html
> 
>   B) With Workaround (Works; do see "Test Succeeded" alert):
> 
>     
> https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src.html
> 
> 
> = = = Code Detail = = = 
> 
>    Compare lines 477 to 482 in 
> 
>      
> https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/js/bug_demo/Element.js 
> 
>    and in
> 
>       https://us.etrade.com/javascript/et1/src/core/dom/Element.js
> 
> 
> = = = Summary = = =  
> 
>      A) Shouldn't at least the "Illegal operation on WrappedNative prototype
> object" be reported as a bug in the dom implementation?  
> 
>      B) Wouldn't the ideal behavior be that exhibited by Safari 3, where for
> example it is very easy to extend 'addEventListener'?  This would help to
> "supercharge" web applications.
> 
> 
>    For more info see:
> http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/4b533df3ccef0c73/ea2f2227d484a4d1?lnk=raot
> 
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1. Go to
> https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src-bug-demo.html
> 2. Click 'OK' on "Proceed with test" alert.
> 3. View error in Error console or in Venkman JavaScript debugger.
> Actual Results:  
> Get this error: 
> 
> wrapping of the 'addEventListener' method failed, and the scope of the event is
> now the DOM object, not the custom application-level JavaScript object
> intended.  
> 
> Wrapping of the 'addEventListener' method failed, and the scope of the event
> handler is now the DOM object, not the custom application-level JavaScript
> object intended.  
> 
> 
> Expected Results:  
> Should see "Test Succeeded" alert dialog box.
> 
> View workaround at:
> 
>     
> https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/index-src.html
> 
> Note that the workaround results in having to modify the instance of every DOM
> element that we would like to use the framework 'addEventListener' on.  Safari
> 3, by contrast, lets us modify just 'Element.prototype' and we are set for all
> elements.  That's an order of magnitude more efficient.

Due to change in test case library, the line range for comparison is different.  Updated:

= = = Code Detail = = = 

   Compare lines 649 to 656 in 

     
https://us.etrade.com/javascript/et1/dev_tools/test/harness/lazy_loader_inline_script/js/bug_demo/Element.js 

   and in

      https://us.etrade.com/javascript/et1/src/core/dom/Element.js

Comment 9

[ChristopherBalz]

( The above mentioned update will be available July 25th ).

Comment 10

[ChristopherBalz]

The W3C Editor's Draft (August 2009) apparently specifies that 'addEventListener' be overridable on a single object, a mixin prototype object: 

  http://dev.w3.org/2006/webapi/WebIDL/#PrototypeRoot

  http://lists.w3.org/Archives/Public/www-dom/2009JulSep/0331.html

This would enable the end result sought by this bug, although via a different means.

Comment 11

[ChristopherBalz]

Per https://bugzilla.mozilla.org/show_bug.cgi?id=428229#c18 , this bug is not a duplicate of 428229.  Per that comment, it is a duplicate of "the bug on maybe
implementing what the HTML5 draft has to say about web prototypes right now" (comment above clarifies that issue).  I cannot seem to find that other bug, and the issue is still not resolved, so re-opening this bug for now.

Comment 12

[Cameron McCormack (:heycam)]

(In reply to comment #10)
> The W3C Editor's Draft (August 2009) apparently specifies that
> 'addEventListener' be overridable on a single object, a mixin prototype object: 
> 
>   http://dev.w3.org/2006/webapi/WebIDL/#PrototypeRoot
> 
>   http://lists.w3.org/Archives/Public/www-dom/2009JulSep/0331.html

I'll caution though that the requirements on prototype chains are some of the more speculative ones in Web IDL, though.  I don't think implementors have reviewed them in depth.  The plan is for the spec to go to LC soon, so maybe once LC review comments are in we can tell whether the current method of collecting all mixin interface properties in to a single mixin prototype object is acceptable.

Comment 14

[John-David Dalton]

Is this still a bug? I tried doing this in Firebug for FF 6 and did not receive an error.

var foo = function() { };
HTMLDivElement.prototype.addEventListener = foo;
console.log(document.createElement('div').addEventListener == foo); // true

Comment 15

[Lea Verou]

(In reply to JDD from comment #14)
> Is this still a bug? I tried doing this in Firebug for FF 6 and did not
> receive an error.
> 
> var foo = function() { };
> HTMLDivElement.prototype.addEventListener = foo;
> console.log(document.createElement('div').addEventListener == foo); // true

If it's not an issue any more, it means my bug (https://bugzilla.mozilla.org/show_bug.cgi?id=686210) got erroneously marked as a duplicate of this, since mine is definitely reproducible in FF6.

Comment 16

[Masatoshi Kimura [:emk]]

Probably fixed by the WebIDL bindings migration. Could you verify that?

Comment 17

[ChristopherBalz]

Works fine for me on "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0":

>>> bar = HTMLDivElement.prototype.addEventListener;
addEventListener()
>>> foo = function() { console.log('bat'); bar(); };
function()
>>> HTMLDivElement.prototype.addEventListener = foo;
function()
>>> HTMLDivElement.prototype.addEventListener()
bat

(There is a TypeError after that, but it is due to not supplying the proper arguments to the 'addEventListener' invocation).

Comment 18

[Masatoshi Kimura [:emk]]

Resolving per the comment.