#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 6 years ago by
Owner: | set to joemabel |
---|---|
Status: | new → pending |
comment:2 Changed 6 years ago by
Resolution: | → duplicate |
---|---|
Status: | pending → closed |
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.
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.