Skip to main content

Search and Top Navigation

#15198 closed bug (duplicate)

Opened June 02, 2017 07:36PM UTC

Closed June 02, 2017 07:45PM UTC

Last modified June 02, 2017 07:50PM UTC

Fails to guard against zero-length array

Reported by: joemabel Owned by: joemabel
Priority: minor Milestone: none
Component: ui.dialog Version: 1.12.1
Keywords: Cc:
Blocked by: Blocking:
Description

Apologies for no jFiddle etc. here, this was in a program of 100,000+ lines of JS; I know exactly the fix needed, it's a simple piece of defensive programming.

In _createOverlay: function(), you have code as follows:

    if ( !this._allowInteraction( event ) ) {
        event.preventDefault();
        this._trackingInstances()[ 0 ]._focusTabbable();
    }

There are definitely circumstances where this._trackingInstances() can be an empty array, so this should be something like:

    if ( !this._allowInteraction( event ) ) {
        event.preventDefault();
        if (this._trackingInstances().length) { 
            this._trackingInstances()[ 0 ]._focusTabbable();
        }
    }

It might be better to put the return of this._trackingInstances() in a local variable to avoid calling it twice; I leave that to you. But it is definitely possible for the array returned to be an empty array, in which case this._trackingInstances()[ 0 ]._focusTabbable() will certainly throw an error in the existing code.

Attachments (0)
Change History (3)

Changed June 02, 2017 07:40PM UTC by scottgonzalez comment:1

owner: → joemabel
status: newpending

Without a use case that creates a problem, we do not considered this a bug. You can't just say "There are definitely circumstances where this._trackingInstances() can be an empty array" and expect it to be taken as truth.

Changed June 02, 2017 07:45PM UTC by scottgonzalez comment:2

resolution: → duplicate
status: pendingclosed

Duplicate of #15182.It would have been better to report the actual bug that you encountered. Compare the difference between the wording in this ticket, which describes what could be a hypothetical problem, to the wording in #15182 which actually states that the error does occur, along with reproduction steps. That ticket was clearly a bug, where this sounded like a theoretical problem. Also note that the fix didn't require us checking for _trackingInstances() returning an empty array, because that's an invalid state.

Changed June 02, 2017 07:48PM UTC by joemabel comment:3

_comment0: Then feel perfectly free not to do obvious defensive programming. I don't understand enough about this to know what circumstances trigger it, and I'm writing as a user. In the jquery-ui code, it's obvious `_trackingInstances()` can return an empty array, and `_createOverlay: function()`, which calls it, does not check for that return. \ \ Your site says you welcome bug reports, particularly with proposed fixes. I've patched it on our system. If no one else cares, then it's not my issue.1496433019057733

[rm comment cross-posted with previous comment]