Thank you for reporting this bug. We will make it our priority to review this report.
  1. #1
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default [4.1.x] IE8 ignoring clicks, sometimes.

    [4.1.x] IE8 ignoring clicks, sometimes.


    Been trying to track down why in IE8 some click events seem to get ignored.
    It's not readily reproducible, I think it relies on air pressure and wind direction or something!
    I can assure you though, it does occur.



    A colleague found some code in Ext.button.Button.onClick where the event is effectively cancelled if e.button !== 0.
    So I thought I'd put a breakpoint in it and take a look at what is going on if/when it occurs.


    Found the following using the watch window:
    Code:
       e.browserEvent                   DispCEventObj (i.e. some Windows COM event object I assume)
       e.browserEvent == true           false
       typeof e.browserEvent            "object"
       Ext.isDefined(e.browserEvent)    true
       Ext.isObject(e.browserEvent)     true

    In EventObject we have lots of "if (event)" tests (see setEvent for instance), which will obviously fail given that e.browserEvent is not truthy (as seen above). That's why e.button is -1, and the event is being cancelled (well, returned).


    I propose changing tests for the event object being set to Ext.isDefined perhaps?


    Thoughts?

    Cheers,
    Westy
    Product Architect
    Altus Ltd.

  2. #2
    Sencha - Senior Forum Manager mitchellsimoens's Avatar
    Join Date
    Mar 2007
    Location
    Gainesville, FL
    Posts
    36,632
    Vote Rating
    817
    mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute mitchellsimoens has a reputation beyond repute

      0  

    Default


    So you mean this line

    Code:
    if (event == me || (event && event.browserEvent)) {
    needs to be like

    Code:
    if (event == me || (event && Ext.isDefined(event.browserEvent))) {
    Mitchell Simoens @SenchaMitch
    Sencha Inc, Senior Forum Manager
    ________________
    Check out my GitHub, lots of nice things for Ext JS 4 and Sencha Touch 2
    https://github.com/mitchellsimoens

    Think my support is good? Get more personalized support via a support subscription. https://www.sencha.com/store/

    Need more help with your app? Hire Sencha Services services@sencha.com

    Want to learn Sencha Touch 2? Check out Sencha Touch in Action that is in print!

    When posting code, please use BBCode's CODE tags.

  3. #3
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Hi Mitchell,

    Yeah, and think there was another one, just after event = event.browserEvent or something in that method.
    Not on my work machine now so can't check, that's the basic idea though.

    I suspect there will be other places in Ext where a browser object is used, that does not equate as truthy when it's an IE COM object...

    Cheers,
    Westy
    Product Architect
    Altus Ltd.

  4. #4
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    It's more than just setEvent, for example:
    Code:
        constructor: function(event, freezeEvent){
            if (event) {
                this.setEvent(event.browserEvent || event, freezeEvent);
            }
        },
    Should become:
    Code:
        constructor: function(event, freezeEvent){
            var e = event;
            if (Ext.isDefined(e)) {
                if (Ext.isDefined(e.browserEvent)) {
                    e = e.browserEvent;
                }
                this.setEvent(e, freezeEvent);
            }
        },
    Quite verbose (and more expensive, since have to set up function call frames which is costly in old IEs), but if event.browserEvent is not truthy then there's not really much more you can do.

    I'll try and come up with a complete override for the class and post it here.

    Cheers,
    Westy
    Product Architect
    Altus Ltd.

  5. #5
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Looks like might be issues in EventManager too.

    Edit: Yeah, lots of:
    Code:
    event = event.browserEvent || event;
    Which you simply cannot do when the object is not truthy.

    Has to become:
    Code:
    event = Ext.isDefined(event.browserEvent) ? event.browserEvent : event;
    Trickier to override the EventManager class too, but will come up with something...
    Product Architect
    Altus Ltd.

  6. #6
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Very confused.

    Am using the following override, and am still seeing e.button !== 0 in button.onClick - it's -1 again!
    Arrgghh!!

    Code:
    Ext.require('Ext.EventObjectImpl',
        function() {
            Ext.override(Ext.EventObjectImpl, {
                /**
                 * Override to guard against DispCEventObj not being truthy in IE8.
                 * See: http://www.sencha.com/forum/showthread.php?246350-4.1.x-IE8-ignoring-clicks-sometimes
                 */
                constructor: function(event, freezeEvent){
                    var e = event;
                    if (Ext.isDefined(e)) {
                        if (Ext.isDefined(e.browserEvent)) {
                            e = e.browserEvent;
                        }
                        this.setEvent(e, freezeEvent);
                    }
                },
    
    
                /**
                 * Override to guard against DispCEventObj not being truthy in IE8.
                 * See: http://www.sencha.com/forum/showthread.php?246350-4.1.x-IE8-ignoring-clicks-sometimes
                 */
                setEvent: function(event, freezeEvent){
                    var me = this, button, options;
    
    
                    if (event == me || (Ext.isDefined(event) && Ext.isDefined(event.browserEvent))) { // already wrapped
                        return event;
                    }
                    me.browserEvent = event;
                    if (Ext.isDefined(event)) {
                        // normalize buttons
                        button = event.button ? me.btnMap[event.button] : (event.which ? event.which - 1 : -1);
                        if (me.clickRe.test(event.type) && button == -1) {
                            button = 0;
                        }
                        options = {
                            type: event.type,
                            button: button,
                            shiftKey: event.shiftKey,
                            // mac metaKey behaves like ctrlKey
                            ctrlKey: event.ctrlKey || event.metaKey || false,
                            altKey: event.altKey,
                            // in getKey these will be normalized for the mac
                            keyCode: event.keyCode,
                            charCode: event.charCode,
                            // cache the targets for the delayed and or buffered events
                            target: Ext.EventManager.getTarget(event),
                            relatedTarget: Ext.EventManager.getRelatedTarget(event),
                            currentTarget: event.currentTarget,
                            xy: (freezeEvent ? me.getXY() : null)
                        };
                    } else {
                        options = {
                            button: -1,
                            shiftKey: false,
                            ctrlKey: false,
                            altKey: false,
                            keyCode: 0,
                            charCode: 0,
                            target: null,
                            xy: [0, 0]
                        };
                    }
                    Ext.apply(me, options);
                    return me;
                },
    
    
                /**
                 * Override to guard against DispCEventObj not being truthy in IE8.
                 * See: http://www.sencha.com/forum/showthread.php?246350-4.1.x-IE8-ignoring-clicks-sometimes
                 */
                preventDefault: function(){
                    if (Ext.isDefined(this.browserEvent)) {
                        Ext.EventManager.preventDefault(this.browserEvent);
                    }
                },
    
    
                /**
                 * Override to guard against DispCEventObj not being truthy in IE8.
                 * See: http://www.sencha.com/forum/showthread.php?246350-4.1.x-IE8-ignoring-clicks-sometimes
                 */
                stopPropagation: function(){
                    var browserEvent = this.browserEvent;
    
    
                    if (Ext.isDefined(browserEvent)) {
                        if (browserEvent.type == 'mousedown') {
                            Ext.EventManager.stoppedMouseDownEvent.fire(this);
                        }
                        Ext.EventManager.stopPropagation(browserEvent);
                    }
                }
            }, function() {
                Ext.EventObject = new Ext.EventObjectImpl();
            });
        }
    );
    Have confirmed that the execution path is now in the if block, rather than the else.

    Will keep digging...
    Product Architect
    Altus Ltd.

  7. #7
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Hmm, and in the case where it's -1 it's not getting set by the else block of setEvent!
    Product Architect
    Altus Ltd.

  8. #8
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Ok, something to do with preventDefault I think.

    Snippet from my button classes onClick:
    Code:
            if (e.button !== 0) {
                Ext.global.console.warn('pre-callParent: Button is non-zero!', e, e.button);
            }
    
    
            this.callParent(arguments);
    
    
            if (e.button !== 0) {
                Ext.global.console.warn('post-callParent: Button is non-zero!', e, e.button);
            }
    I'm seeing the post-callParent log showing up, with a -1.
    Seems that preventDefault is setting the button to -1... which is odd. See it setting the keyCode to -1 sometimes.

    Will keep digging, again...
    Product Architect
    Altus Ltd.

  9. #9
    Ext JS Premium Member westy's Avatar
    Join Date
    Feb 2009
    Location
    Bath, UK
    Posts
    855
    Vote Rating
    38
    westy is a jewel in the rough westy is a jewel in the rough westy is a jewel in the rough

      0  

    Default


    Ok, an update.

    I'm still very concerned by the non-truthy nature of events, but think it's ok. I suspect that setEvent is getting called twice, once with a browserEvent, which goes down the -1 route, and once with an event that wraps it, which is truthy.

    That's my guess anyway. I think it needs attention though.

    Anyway, back to what's happening with me.
    Tracked it down to the fact that, during a click event, I mask the panel that owns the button.
    This seems to change the value of e.button mid-call! Almost like there is only one event object instance or something...

    The upshot is, if I defer the mask by a few milliseconds, it behaves as expected.
    Very strange, and very worrying to be honest...

    So please, rework EventObject and EventManager to be safer since DispCEventObj instances are not truthy
    Product Architect
    Altus Ltd.

  10. #10
    Ext JS Premium Member
    Join Date
    Apr 2008
    Posts
    352
    Vote Rating
    14
    rich02818 is on a distinguished road

      0  

    Default


    Quote Originally Posted by westy View Post
    Ok, an update.

    I'm still very concerned by the non-truthy nature of events, but think it's ok. I suspect that setEvent is getting called twice, once with a browserEvent, which goes down the -1 route, and once with an event that wraps it, which is truthy.

    That's my guess anyway. I think it needs attention though.

    Anyway, back to what's happening with me.
    Tracked it down to the fact that, during a click event, I mask the panel that owns the button.
    This seems to change the value of e.button mid-call! Almost like there is only one event object instance or something...

    The upshot is, if I defer the mask by a few milliseconds, it behaves as expected.
    Very strange, and very worrying to be honest...

    So please, rework EventObject and EventManager to be safer since DispCEventObj instances are not truthy
    It's always dangerous to touch any component (or event object) within the context of an event on the component/object. I've found this true in javascript as well as native compiled languages. The approach you mentioned (ie save sufficient info and defer that actual action itself until outside the event context) is precisely what I've done for years. Don't know if that makes you feel any better or not

Thread Participants: 3

Tags for this Thread