Last Comment Bug 456151 - Cannot wrap DOM element prototype methods 'addEventListener' and 'removeEventListener'
: Cannot wrap DOM element prototype methods 'addEventListener' and 'removeEvent...
Status: RESOLVED WORKSFORME
: testcase
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: unspecified
: x86 Windows XP
: P3 enhancement with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Andrew Overholt [:overholt]
Mentors:
https://us.etrade.com/javascript/et1/...
: 686210 (view as bug list)
Depends on: ParisBindings 428229
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-19 17:18 PDT by ChristopherBalz
Modified: 2013-08-25 15:34 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description User image ChristopherBalz 2008-09-19 17:18:50 PDT
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 User image ChristopherBalz 2008-09-19 17:24:00 PDT
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 User image Jesse Ruderman 2008-09-19 21:37:56 PDT
This might be a dup of bug 428229, but I can't tell because the reporter's testcase is gigantic.
Comment 3 User image ChristopherBalz 2008-09-22 12:09:45 PDT
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 User image Jesse Ruderman 2008-09-22 13:22:02 PDT
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 User image ChristopherBalz 2008-09-23 10:08:19 PDT
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 User image ChristopherBalz 2008-09-23 10:13:33 PDT
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 7 User image Boris Zbarsky [:bz] (still a bit busy) 2008-10-02 11:35:03 PDT

*** This bug has been marked as a duplicate of bug 428229 ***
Comment 8 User image ChristopherBalz 2009-07-06 16:23:50 PDT
(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 User image ChristopherBalz 2009-07-06 16:24:52 PDT
( The above mentioned update will be available July 25th ).
Comment 10 User image ChristopherBalz 2009-09-20 13:39:09 PDT
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 User image ChristopherBalz 2009-09-20 13:52:28 PDT
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 User image Cameron McCormack (:heycam) 2009-09-20 15:34:50 PDT
(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 13 User image Boris Zbarsky [:bz] (still a bit busy) 2011-09-11 08:19:03 PDT
*** Bug 686210 has been marked as a duplicate of this bug. ***
Comment 14 User image John-David Dalton 2011-09-16 14:35:24 PDT
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 User image Lea Verou 2011-09-16 14:37:12 PDT
(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 User image Masatoshi Kimura [:emk] 2013-08-20 02:57:40 PDT
Probably fixed by the WebIDL bindings migration. Could you verify that?
Comment 17 User image ChristopherBalz 2013-08-25 11:14:51 PDT
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 User image Masatoshi Kimura [:emk] 2013-08-25 15:34:13 PDT
Resolving per the comment.

Note You need to log in before you can comment on or make changes to this bug.