Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#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)

test12257.html (566 bytes) - added by Kenneth G. Franqueiro 7 years ago.
Simple test to reproduce the issue
test12257.2.html (1.2 KB) - added by Kenneth G. Franqueiro 7 years ago.
Slightly different / more elaborate test file with some more cases
test12257.3.html (1.6 KB) - added by Kenneth G. Franqueiro 7 years ago.
Added keypress/keydown tests
12257.diff (1.1 KB) - added by Kenneth G. Franqueiro 7 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 7 years ago by Kenneth G. Franqueiro

Attachment: test12257.html added

Simple test to reproduce the issue

comment:1 Changed 7 years ago by Kenneth G. Franqueiro

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 7 years ago by Kenneth G. Franqueiro

Attachment: test12257.2.html added

Slightly different / more elaborate test file with some more cases

comment:2 Changed 7 years ago by Kenneth G. Franqueiro

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 7 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 7 years ago by Kenneth G. Franqueiro

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 7 years ago by Kenneth G. Franqueiro

Attachment: test12257.3.html added

Added keypress/keydown tests

comment:5 Changed 7 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 7 years ago by Kenneth G. Franqueiro

Attachment: 12257.diff added

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

comment:6 Changed 7 years ago by Kenneth G. Franqueiro

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 7 years ago by Eugene Lazutkin

Milestone: tbdfuture
Owner: changed from sjmiles to Eugene Lazutkin
Status: newassigned

comment:8 Changed 7 years ago by Kenneth G. Franqueiro

Owner: changed from Eugene Lazutkin to Kenneth G. Franqueiro
Status: assignednew

comment:9 Changed 7 years ago by Kenneth G. Franqueiro

Milestone: future1.6
Status: newassigned

comment:10 Changed 7 years ago by Kenneth G. Franqueiro

Resolution: fixed
Status: assignedclosed

(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 6 years ago by Kenneth G. Franqueiro

Milestone: 1.61.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.