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 comment:1
owner: | → joemabel |
---|---|
status: | new → pending |
Changed June 02, 2017 07:45PM UTC by comment:2
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.
Changed June 02, 2017 07:48PM UTC by 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]
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.