Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#12257 closed defect (fixed)

IE9: dojo.stopEvent doesn't completely stop events

Reported by: kgf Owned by: kgf
Priority: high Milestone: 1.5.2
Component: Events Version: 1.6.0b1
Keywords: ie9 stopevent Cc:
Blocked by: Blocking:

Description

This issue was actually reported on StackOverflow: http://stackoverflow.com/questions/4897914/whats-the-best-way-to-stop-a-click-event-propagating-in-ie9-beta

dojo.stopEvent doesn't seem to fully do its job in IE9. The questionable code is within an IE-specific code path in dojo/_base/event.js from line 307 to 508. This would be a *lot* of code to simply short-circuit for IE9, and moreover, doing so doesn't seem to fix this issue anyway.

(Interestingly, this is also where the code pertinent to #11657 would have been as well, if that problem didn't seemingly go away by Microsoft's own doing.)

Attachments (4)

test12257.html (566 bytes) - added by kgf 3 years ago.
Simple test to reproduce the issue
test12257.2.html (1.2 KB) - added by kgf 3 years ago.
Slightly different / more elaborate test file with some more cases
test12257.3.html (1.6 KB) - added by kgf 3 years ago.
Added keypress/keydown tests
12257.diff (1.1 KB) - added by kgf 3 years ago.
First attempt at a minimal changeset to event.js to fix stopPropagation in IE9.

Download all attachments as: .zip

Change History (15)

Changed 3 years ago by kgf

Simple test to reproduce the issue

comment:1 Changed 3 years ago by kgf

Update: I realize now that there are *several* isIE checks in dojo/_base/event.js - if I revise both lines 307 and 620 to check isIE < 9, this bug seems to go away, but I have no idea what kind of can of worms this would open.

Changed 3 years ago by kgf

Slightly different / more elaborate test file with some more cases

comment:2 Changed 3 years ago by kgf

Just started testing with the official RC released today, and maybe I'm seeing ghosts, but I think I'm getting somewhat fewer spurious results (fewer differences in behavior between IE8 and IE9 standards modes). However, oncontextmenu particularly still seems to bleed through (which is the cause of #12256 which maybe I should close as a duplicate at this point).

comment:3 Changed 3 years ago by bill

I would vote for you to check in your fix to event.js, even though we can't fully test it yet, at least it fixes this problem.

comment:4 Changed 3 years ago by kgf

Just to follow up on my previous comment, in this case I think I was indeed seeing ghosts - with my latest test page, same results on both versions I checked. The issue seems to really be with preventDefault only; stopPropagation doesn't seem to be an issue.

I just did a bit of key event testing - as I feared, incorporating such wide codepath changes for IE9 has regression issues - for one, the code that attempts to make keydown/keypress event firing consistent is still needed in IE9 (IOW, keydown fires on unprintable characters, but keypress doesn't). I think I'm going to need to go for a far more specific alteration if possible.

Changed 3 years ago by kgf

Added keypress/keydown tests

comment:5 Changed 3 years ago by bill

Ah too bad, guess I was wishful thinking. BTW there's an eventKeyPressRobot.html test file, which controls the eventKeyPress.html manual test file. It isn't run as part of runTests.html (just because dojo/tests/runTests.html doesn't run any robot test files), but it's worth running once we have a prospective fix for events.js on IE9, and robot is working on IE9.

Changed 3 years ago by kgf

First attempt at a minimal changeset to event.js to fix stopPropagation in IE9.

comment:6 Changed 3 years ago by kgf

D'oh, the message on that attachment should've said preventDefault technically, not stopPropagation.

Also forgot to mention, this changeset also updates the codepath for indicating which mouse button is clicked, since IE9 seems to match the standard rather than IE's previous behavior of using "stackable" button codes.

I'm thinking at the very least, the edit around L427 is suboptimal, since that condition would end up evaluated on every call to _fixEvent. Not sure what the optimal solution is yet.

Also note that the dojo.isIE < 9 || dojo.isQuirks checks in the changeset should be sufficient since they are already within a dojo.isIE code branch.

comment:7 Changed 3 years ago by elazutkin

  • Milestone changed from tbd to future
  • Owner changed from sjmiles to elazutkin
  • Status changed from new to assigned

comment:8 Changed 3 years ago by kgf

  • Owner changed from elazutkin to kgf
  • Status changed from assigned to new

comment:9 Changed 3 years ago by kgf

  • Milestone changed from future to 1.6
  • Status changed from new to assigned

comment:10 Changed 3 years ago by kgf

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [23802]) Refine code paths in event.js to avoid breaking dojo.stopEvent (and preventDefault) in IE9, which should just work. Fixes #12257 !strict

comment:11 Changed 2 years ago by kgf

  • Milestone changed from 1.6 to 1.5.2

Updating milestone to 1.5.2 to reflect inclusion in changeset [26956] for ticket #14199.

Note: See TracTickets for help on using tickets.