[CLOSED] Exception when selecting Quick Open result by clicking

[core-ai-bot]

Issue by peterflynn
Thursday Aug 16, 2012 at 18:54 GMT
Originally opened as adobe/brackets#1384

---

1. Launch "Quick Open" (Ctrl+Shift+O)
2. Click any of the items in the result list

Result: exception in jquery.smart-autocomplete.js

Expected: no exception -- it worked fine a few sprints ago

Alternative repro steps:

1. Launch "Go to Line" (Ctrl+G / Cmd+L)
2. Type a line number and quickly press Enter before the editor scrolling has fully caught up

[core-ai-bot]

Comment by peterflynn
Thursday Aug 16, 2012 at 18:57 GMT

---

Surprisingly, it looks like this was caused by 0159dc2. Removing a DOM node with jQuery's remove(), instead of the generic DOM API removeChild(), has the side effect of stripping off the node's stored jQuery data bag. When we close the dialog this way, the smart-autocomplete itemSelected handler runs a moment later and crashes on the missing data. (Or in the Go to Line case, it's the resultsReady handler that runs asynchronously a moment later still).

It seems like we can't backtrack to the old removal, though, because jQuery basically requires all removals to use its own APIs -- it actually leaks memory (everything you stored on the node(s) via .data()) otherwise.

[core-ai-bot]

Comment by pthiess
Tuesday Aug 21, 2012 at 18:19 GMT

---

Reviewed - Peter has a solution.

[core-ai-bot]

Comment by redmunds
Tuesday Aug 28, 2012 at 21:58 GMT

---

I am seeing 2 exceptions:

1. jquery.smart-autocomplete.js line 348. This only seems to happen when Quick Open is called with no other files open. This seems to be fixed by upstream fix fix the issue when you select something using up/down keys and then use mouse to select a different one laktek/jQuery-Smart-Auto-Complete#21. It looks like we submodule that repo directly, so I'm not sure about updating our submodule for that fix (since it could also bring another change).
2. jquery.smart-autocomplete.js line 306. This seems to only be caused when selecting file in Quick Open list using mouse. A simple check for options being defined fixes this one.

[core-ai-bot]

Comment by peterflynn
Wednesday Aug 29, 2012 at 09:18 GMT

---

Both are a side effect of 0159dc2 -- the smart-auto-complete plugin isn't expecting its metadata to be cleared before it detaches all its listeners, which I think is just a special case of it not expecting the popup to be closed by anyone other than itself. It relies on global focus & click events to decide when to close the popup, which is problematic for us because we hide the popup in other cases too, like key events.

We weren't seeing these bugs before 0159dc2 only because because we were never clearing the metadata (leaking memory).

My original fix was to adjust the timing of when the metadata is removed until after smart-autocomplete received the focus loss event that makes it decide to agree with us on closing the popup. But this doesn't fix Randy's case #1 since EditorManager.focusEditor() is a no-op when there's no editor to focus (and jQuery apparently doesn't dispatch focus loss events when removing an item from the DOM removes its focus -- which seems like a bug to me).

Anyway, I think the real right fix is to explicitly tell smart-autocomplete to close the popup itself before removing the text field from the DOM. I don't think it has an official API for this, but I think I see a way to trick it. I'll take a look tomorrow.

[core-ai-bot]

Comment by peterflynn
Thursday Aug 30, 2012 at 01:33 GMT

---

Fix has been merged -- closing