Opened 22 months ago

Closed 22 months ago

Last modified 22 months ago

#15198 closed bug (duplicate)

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.

Change History (3)

comment:1 Changed 22 months ago by Scott González

Owner: set to 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.

comment:2 Changed 22 months ago by Scott González

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.

comment:3 Changed 22 months ago by joemabel

[rm comment cross-posted with previous comment]

Last edited 22 months ago by joemabel (previous) (diff)
Note: See TracTickets for help on using tickets.