BUG Make UI widget destroys more consistent to avoid exceptions. #3286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Probably should use |
|
Don't merge yet, still testing. |
|
Ok, I think it's ready. |
| }, | ||
|
|
||
| onunmatch: function() { | ||
| this._super(); |
There was a problem hiding this comment.
Is there any benefit to this? It does nothing.
There was a problem hiding this comment.
Yes. It makes onmatch work. If onmatch is present, but onunmatch isn't, onmatch might short-circuit bypassing some handlers.
|
Removed that fine example of a cargo-cult programming. |
| }, | ||
|
|
||
| onunmatch: function() { | ||
| this._super(); |
There was a problem hiding this comment.
Wasn't our revelation that onunmatch relies on onmatch, but not the other way around?
|
Fixed up. |
|
Perfect. Will merge, but can you please squash your commits? |
1. Add missing _super calls. 2. Make UI widget destroys more consistent to avoid exceptions. Selectable would throw an exception in the GridField.js if destroy called from onunmatch - at that stage jQuery UI would have had called the destroy already. Add a guard, and change to onremove, which triggers before the element is removed from DOM. 3. DOM traversal fails after the element is removed from DOM. Onunmatch triggers after the removal of the element from the DOM, which makes DOM traversal fail. Use onremove instead, which triggers while the element is still in DOM.
|
'ere, squished. |
|
Ok, hope all the tests pass. |
tractorcow
pushed a commit
that referenced
this pull request
Jul 17, 2014
BUG Make UI widget destroys more consistent to avoid exceptions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Selectable would throw an exception in the GridField.js if destroy
called from onunmatch - at that stage jQuery UI would have had called
the destroy already. Add a guard, and change to onremove, which triggers
before the element is removed from DOM.