Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

focus event ignores additional data when triggered #1741

Closed
mgol opened this Issue Oct 21, 2014 · 27 comments

Comments

Projects
None yet
10 participants
@mgol
Copy link
Member

commented Oct 21, 2014

Originally reported by mail@… at: http://bugs.jquery.com/ticket/13428

 http://fiddle.jshell.net/rvX5Y/4/

$('input').on('focus', function(event, extra, data) {
    console.log(extra, data);
});
$('input').trigger('focus', ['extra', 'data']);

Works in version 1.8.3.

Does not work in version 1.9.0 and 1.9.1.

Expected behavior: Console shows "extra data"

False Behavior: Console shows "undefined undefined"

Browser: Firefox 18, OS: OS X 10.8.2

Issue reported for jQuery 1.9.1

Blocked by: #13353

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: gibson042

This is exactly analogous to #13353... I'm starting to think we should generalize and incorporate that  solution after all.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

Way too much code though. I was thinking that if the user passed data that we use the "standard" trigger path rather than native events. It passes the data at the expense of some non-standard event behavior, but I think people are probably misusing the event system most of the time when this happens.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: gibson042

That'd probably be acceptable for focus, but click handlers would see different checked states on checkbox and radio inputs when the event was provided trigger data—a definite no-go.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

So, to the OP, are you using this to initialize on page load? Could you use .triggerHandler() instead, or are you really trying to focus the element as well?

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: mail@…

I was trying to distinguish between a) clicking or tabbing into the input field and b) using the .trigger('focus') function somewhere else. Nothing on page load.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

#13590 is a duplicate of this ticket.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

#13711 is a duplicate of this ticket.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: anonymous

triggerHandler is not really an acceptable workaround since it won't actually focus, blur, etc...

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

It is a fine workaround. Just do the indicated DOM action afterwards.

var $x = $("#aninput");
$x.triggerHandler("focus");
$x[0].focus();
@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: anonymous

People from the core team certainly know this already, but this arises from:

focus: {
  // Fire native event if possible so blur/focus sequence is correct
  trigger: function() {
    if ( this !== safeActiveElement() && this.focus ) {
      this.focus();

and

blur: {
  trigger: function() {
  if ( this === safeActiveElement() && this.blur ) {
    this.blur();
@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: anonymous

Won't this trigger the handlers twice?

Replying to dmethvin:

It is a fine workaround. Just do the indicated DOM action afterwards.

var $x = $("#aninput");
$x.triggerHandler("focus");
$x[0].focus();
@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: osmestad

I also have this issue and it is still valid with latest 1.x and 2.x, here is a jsFiddle for what I'm doing:  http://jsfiddle.net/2dVMa/

My problem with using triggerHandler is that I am trying to avoid running the code in my 'focus' handler when setting focus programatically (with 'trigger'). (Only workarounds I can think of would be unbinding, triggering focus and then rebinding, or reverting to 1.8... )

Others with same issue:  http://forum.jquery.com/topic/extra-arguments-passed-to-trigger-doesn-t-reach-blur-event-handler-when-element-has-focus

Browser: Chrome 27, OS: OS X 10.8.4

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

#14658 is a duplicate of this ticket.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2014

Comment author: dmethvin

#14829 is a duplicate of this ticket.

@netil

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2015

For 'focus', 'blur' and 'click' events, it fires native event in certain condition and when
calling native event, currently there is no way to pass through trigger's data to event handler.

Well, is not the best solution but I though to use special event hook to handle that.
Need to modify trigger to hold the data, and add handle method to invoke event handler function
(obviously it can be generalized to cover other events too).

What do you think guys?

        focus: {
            // Fire native event if possible so blur/focus sequence is correct
            trigger: function() {
                if ( this !== safeActiveElement() && this.focus ) {

                    // Check if triggered with data
                    if( arguments.length > 1 ) {
                        jQuery.event.special.focus.triggerData = jQuery.makeArray(arguments).splice(1);
                    }

                    this.focus();
                    return false;
                }
            },
            delegateType: "focusin",
            handle: function( event ) {
                var param = [ event ],
                    data = jQuery.event.special.focus.triggerData;

                if( data ) {
                    param = param.concat( data );
                }

                event.handleObj.handler.apply( this, param );
                delete jQuery.event.special.focus.triggerData;
            }
        }
@gibson042

This comment has been minimized.

Copy link
Member

commented Jul 6, 2015

That solution is at risk for introducing strange behavior flips between the "no arguments" and "arguments" cases, and is not reentrant safe. I don't see us using such an approach.

@dmethvin

This comment has been minimized.

Copy link
Member

commented Jul 6, 2015

Agreed. In retrospect, allowing users to pass data has caused a lot of issues and this is one of them. Focus management is highly flaky in browsers already, it doesn't take much to cause problems.

@mgol

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2015

@timmywil Do we still want it for 3.0.0? There wasn't a lot of progress and we need to cut corners.

@mgol mgol modified the milestones: 3.1.0, 3.0.0 Sep 15, 2015

@mgol

This comment has been minimized.

Copy link
Member Author

commented Sep 15, 2015

Moving to 3.1.0.

@dmethvin dmethvin removed this from the 3.2.0 milestone Sep 26, 2016

@scholtzm

This comment has been minimized.

Copy link

commented Nov 9, 2016

I'm dealing with the same situation as described in this comment.

...
My problem with using triggerHandler is that I am trying to avoid running the code in my 'focus' handler when setting focus programatically (with 'trigger').
...

Is there a proper way to solve this?

@obskyr

This comment has been minimized.

Copy link

commented Nov 17, 2016

Welp, I just ran into this. Tried to handle a blur event only if triggered with a certain parameter, but I guess they just don't get passed. There's a use case, at least!

@PurvaA

This comment has been minimized.

Copy link

commented Nov 23, 2016

@mgol

There is an another way to show additional data when focus event triggered

Please check my fiddle - https://fiddle.jshell.net/426vdp05/1/

I am attaching screen shot for same

focus_issue

@mgol

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2016

@PurvaA You're not triggering the focus event by yourself in your fiddle and you don't pass any data so this doesn't seem relevant to this issue.

@pushkin-

This comment has been minimized.

Copy link

commented Jan 12, 2018

This is still broken. Is this issue being tracked in one of the future milestones?

@dmethvin

This comment has been minimized.

Copy link
Member

commented Jan 12, 2018

Yes, the ticket is still open and the milestone is 4.0.0, scroll to the top of this ticket and it should say that. If you'd like to submit a PR we can review.

@caseyjhol

This comment has been minimized.

Copy link

commented Sep 15, 2018

This might be an acceptable workaround to prevent firing the handler twice as mentioned in #1741 (comment):

var $x = $("#aninput");
$x.triggerHandler("focus", ['extra', 'data']);
$x[0].focus();

$x.on("focus", function (e, extra, data) {
    if (e.originalEvent) return;
    console.log(extra, data);
});
@mgol

This comment has been minimized.

Copy link
Member Author

commented Mar 20, 2019

Changing the milestone, we'll fix that in 3.4.0.

@mgol mgol closed this in 669f720 Mar 20, 2019

@GulajavaMinistudio GulajavaMinistudio referenced this issue Mar 21, 2019

Merged

Bottom sheet scrolling #79

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.