Skip to main content

Search and Top Navigation

#15252 new bug ()

Opened November 28, 2017 03:12AM UTC

Last modified November 28, 2017 03:14AM UTC

Resizable is too greedy during create

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

This is fairly straightforward and easy to spot. In fact, a similar bug was fixed at some point in the same function. i don't see any open bugs for it, nor any PRs on Github. It doesn't require anything special to reproduce.

In the Resizable _setupHandles() function, this line (322):

this._handles = this._handles.add( this.element.find( ".ui-resizable-handle" ) );

The find() call makes the widget too greedy. A previous change converted an element selection using context into a more specific children() selection:

this.handles[ i ] = this.element.children( this.handles[ i ] ).first().show();

// Used to be something like
this.handles[i] = $(this.handles[i], this.element);

and the line above should be doing the equivalent.

Demonstration of the problem is fairly easy: create one resizable inside another, initializing the inner one first. On the outer one, set the autoHide option, which is dealt with after this._handles is collected in the problem line above.

https://jsfiddle.net/7y0g0fde/1/

Additional side effects include sending extra descendant nodes from interior Resizables through disableSelection() and adding a redundant mouseover handler. The more nested Resizables, the worse this gets.

The fix should be very simple (change find() to children()), so if this issue is verified, i can submit a PR for it, or someone who's already signed on as a contributor can take it.

Attachments (0)
Change History (1)

Changed November 28, 2017 03:14AM UTC by cautionbug comment:1

description: This is fairly straightforward and easy to spot. In fact, a similar bug was fixed at some point in the same function. i don't see any open bugs for it, nor any PRs on Github. It doesn't require anything special to reproduce. \ \ In the Resizable `_create()` function, this line (322): \ {{{ \ this._handles = this._handles.add( this.element.find( ".ui-resizable-handle" ) ); \ }}} \ The `find()` call makes the widget too greedy. A previous change converted an element selection using context into a more specific `children()` selection: \ {{{ \ this.handles[ i ] = this.element.children( this.handles[ i ] ).first().show(); \ \ // Used to be something like \ this.handles[i] = $(this.handles[i], this.element); \ }}} \ and the line above should be doing the equivalent. \ \ Demonstration of the problem is fairly easy: create one resizable inside another, initializing the inner one first. On the outer one, set the `autoHide` option, which is dealt with after `this._handles` is collected in the problem line above. \ https://jsfiddle.net/7y0g0fde/1/ \ \ Additional side effects include sending extra descendant nodes from interior Resizables through `disableSelection()` and adding a redundant `mouseover` handler. The more nested Resizables, the worse this gets. \ \ The fix should be very simple (change `find()` to `children()`), so if this issue is verified, i can submit a PR for it, or someone who's already signed on as a contributor can take it.This is fairly straightforward and easy to spot. In fact, a similar bug was fixed at some point in the same function. i don't see any open bugs for it, nor any PRs on Github. It doesn't require anything special to reproduce. \ \ In the Resizable `_setupHandles()` function, this line (322): \ {{{ \ this._handles = this._handles.add( this.element.find( ".ui-resizable-handle" ) ); \ }}} \ The `find()` call makes the widget too greedy. A previous change converted an element selection using context into a more specific `children()` selection: \ {{{ \ this.handles[ i ] = this.element.children( this.handles[ i ] ).first().show(); \ \ // Used to be something like \ this.handles[i] = $(this.handles[i], this.element); \ }}} \ and the line above should be doing the equivalent. \ \ Demonstration of the problem is fairly easy: create one resizable inside another, initializing the inner one first. On the outer one, set the `autoHide` option, which is dealt with after `this._handles` is collected in the problem line above. \ https://jsfiddle.net/7y0g0fde/1/ \ \ Additional side effects include sending extra descendant nodes from interior Resizables through `disableSelection()` and adding a redundant `mouseover` handler. The more nested Resizables, the worse this gets. \ \ The fix should be very simple (change `find()` to `children()`), so if this issue is verified, i can submit a PR for it, or someone who's already signed on as a contributor can take it.