#1142 ✓resolved
nombx

IE9 Event Bug

Reported by nombx | September 18th, 2010 @ 05:10 AM | in 1.7

hi,
today i download the ie9 beta;
I am useing Prototype 1.6.1, also using Scriptaculous 1.8 preview.
Beyond changing my "contentloaded" to "dom:loaded" the transition
seemed to go fairly smoothly. Until I encountered the following error
in IE 9:
Drags created with "new Draggable(...)" no longer drag.
I traced this back to prototype's "isLeftClick" always returning false;

Comments and changes to this ticket

  • Juriy Zaytsev

    Juriy Zaytsev September 20th, 2010 @ 06:12 AM

    • Importance changed from “” to “Low”

    The problem here is in the way event abstraction is defined in Prototype.

    When user agent identifies itself as Internet Explorer, Prototype makes few assumptions about that agent's event model — assumptions which no longer hold true in Internet Explorer 9. One of them is presence of such methods and properties on event objects as preventDefault and returnValue.

    For example, when Prototype.Browser.IE evaluates to true, Prototype assumes that event objects have returnValue boolean property (part of event model from IE<9), but not preventDefault method (part of DOM L2 event model). Internet Explorer 9, however, uses DOM L2 event model [*], and so Prototype ends up overwriting event's native preventDefault method with a shim one. The shim obviously assumes that "old" event model is in effect, and is defined as function(){ this.returnValue = false; } (where this references event object). returnValue = false no longer works in DOM L2 event model (which is correct behavior), and preventDefault in IE9 becomes defunct.

    The isLeftClick failure you're describing is part of the same problem — event model is assumed to be that used by IE<9, when user agent identifies itself as IE.

    All of this is, of course, a result of user agent sniffing coupled with unsafe assumptions about user agent capabilities. This wouldn't happen if feature detection was used instead — something I've been evangelizing over and over again for the past couple of years.

    But there's a bit more to it when it comes to Prototype, and the fix is not as simple as replacing sniff with a feature test.

    The problem is that Prototype defines all of its event methods at load time, when event object is not available for testing. This is contrary to, for example, jQuery, which never augments event object prototypes and tests for presence of preventDefault/returnValue properties at run time, directly on an event object passed to a corresponding method. If Prototype was to use event wrappers — which also happen to be superior to event objects' augmentation in few other ways — this problem would be easily solved. This is the best long-term fix, but would require quite substantial changes to Prototype's event abstraction.

    Another way to test event objects could be to generate them manually (e.g. using document.createEvent, etc.), but that would only complicate things; the initialization of an entire event abstraction would now rely on presence of event-creation methods and their proper functionality.

    Yet another longish fix is to change event methods in Prototype — stop, element, isLeftClick, etc. — to test event object's properties at run time, rather than at load time. Optionally, methods could be dynamically overwritten after an initial test.

    The short term — and the least reliable — fix would probably be to augment Prototype.Browser.IE check with addition of document.documentMode < 9.

    if (Prototype.Browser.IE && document.documentMode < 9) {
      ...
    }
    

    [*] when event listeners are attached via addEventListener, not attachEvent

  • Andrew Dupont

    Andrew Dupont September 21st, 2010 @ 06:40 AM

    • Milestone set to 1.7
    • State changed from “new” to “bug”
    • Assigned user set to “Andrew Dupont”
    • Importance changed from “Low” to “High”

    We'll have this fixed for the upcoming release candidate.

  • Mattiacci

    Mattiacci October 5th, 2010 @ 09:48 PM

    I noticed the commit to feature detect IE's legacy event system, and I've an observation: This change will make it so legacy events cannot be extended in IE9. Is this intended?

    The library will add event listeners with addEventListener, so this wouldn't cause a problem there...but won't there be code out there that will no longer function in IE9 (e.g. "if IE window.attachEvent")? That said, performance would be impacted if we had to check each event at run time.

  • Andrew Dupont

    Andrew Dupont October 7th, 2010 @ 09:05 AM

    • Importance changed from “High” to “Medium”

    I wouldn't say it's intended, but that's the way things are at the moment. I doubt there are many people who assign events manually (with attachEvent) and then expect to use Prototype's convenience methods on them, but I'm sure there are a few people who use DOM0-style handlers (someNode.onclick = function() {}) and manually call Event.extend on them.

    It would be nice to get that working, but you're right that it would require a runtime inspection of the event inside Event.extend (instead of just defining it as Prototype.K). As long as it's a one-line check (e.g., testing for a property), I wouldn't be against adding this.

    Leaving this bug open, but downgrading its priority.

  • Mattiacci

    Mattiacci October 7th, 2010 @ 09:44 PM

    I have it working, as long as checking event.stop (instead of event._extendedByPrototype) to determine whether an event has been extended isn't a problem.

    The mouse button handling also needed to be altered to fix the button mapping for legacy events.

    I've attached a diff in case it's useful.

  • Mattiacci

    Mattiacci October 8th, 2010 @ 03:21 AM

    Sorry about that - I attached the wrong version. Here's the correct one.

    If you want better performance in the older IEs, the mouse handling code could be altered so the "window.MouseEvent" check isn't made every time - it would be more code, though.

  • Andrew Dupont

    Andrew Dupont October 20th, 2010 @ 01:57 AM

    Thanks, Mattiacci.

    Here's my attempt at this. It was less painful than I had feared. I commented the hell out of it in order to make it clearer what's going on. Also added a functional test that verifies (a) legacy events can be extended; (b) button mapping works properly.

    Posting it before I commit so I can get some feedback from other core team members.

  • Andrew Dupont

    Andrew Dupont November 2nd, 2010 @ 07:46 AM

    • State changed from “bug” to “resolved”

    Committed in 1a5f04.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Tags

Pages