Bug Tracker

#15073 closed bug (migrated)

Opened May 02, 2014 08:41AM UTC

Closed October 21, 2014 12:42AM UTC

Infinite Loop in .off() with empty namespace

Reported by: Christian Meixner <christian@bippesbrandao.de> Owned by: dmethvin
Priority: low Milestone: 1.12/2.2
Component: event Version: 1.11.0
Keywords: Cc:
Blocked by: Blocking:
Description

IF there is an event registered with namespace and you try to remove this with an an accidentally empty namespace-string, then .off() enters an inifinite loop, locking up the browser window (at least on Firefox).

Example:

$(document).on('click.a', handler),

$(document).off('click.');

Note the trailing dot after "click"!

I am aware that this is not valid code, but it should not fail with an infinite loop blocking the browser.

Attachments (0)
Change History (6)

Changed May 02, 2014 11:51AM UTC by dmethvin comment:1

component: unfiled → event
milestone: None → 1.12/2.2
priority: undecided → low
status: new → open

Yeah, an infinite loop seems like a bad outcome. I suppose it should just ignore a trailing dot as if there were no namespaces at all.

Changed May 05, 2014 04:08PM UTC by dmethvin comment:2

owner: → dmethvin
status: open → assigned

Changed August 01, 2014 05:49PM UTC by hamishdickson comment:3

_comment0: I've created a quick jsFiddle for this here http://jsfiddle.net/EJTyW/5/ \ \ Christian is right, this is a problem in Firefox but Chrome and Safari both handle it (in that they don't loop forever) \ \ I will have a look and see what we can do about it → 1406916067449617

For completeness I've created a quick jsFiddle for this here http://jsfiddle.net/EJTyW/5/

Christian is right, this is a problem in Firefox but Chrome and Safari both handle it (in that they don't loop forever)

I will have a look and see what we can do about it

Changed August 01, 2014 09:12PM UTC by hamishdickson comment:4

_comment0: After some digging about, this actually looks to be an issue with remove(), rather than off(). So it could cause problems elsewhere. \ \ If we go with the idea that it should just ignore a trailing dot as if there were no namespaces at all, then it's just a case of stripping out any trailing dots in types[t] \ \ {{{ \ ... \ \ // Once for each type.namespace in types; type may be omitted \ types = ( types || "" ).match( rnotwhite ) || [ "" ]; \ t = types.length; \ while ( t-- ) { \ types[t] = types[t].replace(/.+$/, ""); \ tmp = rtypenamespace.exec( types[t] ) || []; \ type = origType = tmp[1]; \ namespaces = ( tmp[2] || "" ).split( "." ).sort(); \ \ ... \ }}} \ \ I've run the unit tests over this and I don't get any errors. I also can't think of a reason why it would be valid to have a trailing dot. I will do a proper test tomorrow and submit a pull request unless anyone has any comments on style or logic → 1406970016135952

After some digging about, this actually looks to be an issue with remove(), rather than off(). So it could cause problems elsewhere.

If we go with the idea that it should just ignore a trailing dot as if there were no namespaces at all, then it's just a case of stripping out any trailing dots in types before it gets to the while loop

    ...

    // remove any trailing dots
    types = types.replace(/\\.+\\s/, " ").replace(/\\.+$/, ""); 

    // Once for each type.namespace in types; type may be omitted
    types = ( types || "" ).match( rnotwhite ) || [ "" ];
    t = types.length;
    while ( t-- ) {
	tmp = rtypenamespace.exec( types[t] ) || [];

    ...

I've run the unit tests over this and I don't get any errors. I also can't think of a reason why it would be valid to have a trailing dot. I will do a proper test tomorrow and submit a pull request unless anyone has any comments on style or logic

Changed August 02, 2014 12:46PM UTC by hamishdickson comment:5

_comment0: Pull request #1636 created for fix → 1406983754886712

Pull request 1636 created for fix

Changed October 21, 2014 12:42AM UTC by m_gol comment:6

resolution: → migrated
status: assigned → closed

Migrated to https://github.com/jquery/jquery/issues/1769