#12257 closed defect (fixed)
IE9: dojo.stopEvent doesn't completely stop events
Reported by: | Kenneth G. Franqueiro | Owned by: | Kenneth G. Franqueiro |
---|---|---|---|
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)
Change History (15)
Changed 11 years ago by
Attachment: | test12257.html added |
---|
comment:1 Changed 11 years ago by
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 11 years ago by
Attachment: | test12257.2.html added |
---|
Slightly different / more elaborate test file with some more cases
comment:2 Changed 11 years ago by
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 11 years ago by
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 11 years ago by
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.
comment:5 Changed 11 years ago by
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 11 years ago by
Attachment: | 12257.diff added |
---|
First attempt at a minimal changeset to event.js to fix stopPropagation in IE9.
comment:6 Changed 11 years ago by
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 11 years ago by
Milestone: | tbd → future |
---|---|
Owner: | changed from sjmiles to Eugene Lazutkin |
Status: | new → assigned |
comment:8 Changed 11 years ago by
Owner: | changed from Eugene Lazutkin to Kenneth G. Franqueiro |
---|---|
Status: | assigned → new |
comment:9 Changed 11 years ago by
Milestone: | future → 1.6 |
---|---|
Status: | new → assigned |
comment:10 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:11 Changed 11 years ago by
Milestone: | 1.6 → 1.5.2 |
---|
Simple test to reproduce the issue