JS Code Hint UI stops inserting hints on enter key

[dangoor]

This appears to be a new problem in sprint 32. I see this behavior consistently if I follow the steps in #4991 using the current master b83eeee, but not using Sprint 31.

What I'm seeing is that after I've switched back and forth between the files (I use ctrl-tab on Mac to do it) while following along with the 4991 instructions, at the end pressing enter inserts a newline rather than the code hint.

[dangoor]

I'll note also that I've seen this a few times over the past couple of days, but originally chalked it up to things I had been doing in the code hinting code.

I'll bisect.

[njx]

Ditto--I ran into this once but was unable to reproduce.

[marcelgerber]

I ran into this problem a few times, too (once I got it 3 times in a row). I haven't really changed anything in the core code.

[njx]

@RaymondLim pointed out that it might be related to #5228. If so, we might just want to back that out for now. /cc @redmunds

[dangoor]

bisects to: 4d8924c

[njx]

That commit is a little weird, since it's a merge that includes the changes from #5228 with some other stuff, so this might just be bisect getting confused. Probably worth isolating the changes from #5228 and seeing if you can repro before/after those changes.

[peterflynn]

For the record, the PR for that commit is #3406: "Fix for Issue 1617: Removed all references to CodeHintManager from Editor"

[njx]

Ah, interesting. It could be that there's some bad interaction between that old PR (which was recently merged) and @redmunds' changes.

[WebsiteDeveloper]

I tried to reproduce this, and found out, that it reproduced every time i was typing a little faster, and then pressed Enter.

[marcelgerber]

Not only, I got it when the dropdown showed about 2 seconds, too. But I can't really repro it...

[WebsiteDeveloper]

me too.
sometimes it happens even when i wait and sometimes it doesn't
The only reproducible case i have is the one described above.

[njx]

Inspecting #3406, it looks like it changed Editor to dispatch an editorChange event, but made CodeHintManager listen for a change event on the active editor. Could that be the problem?

[WebsiteDeveloper]

@njx that seems to be it. Must have been missed while merging^ totally my fault :(

[dangoor]

I tried the trivial change of listening for editorChange and it did not appear to fix the problem, so I put up a pull request (#5415) that reverts #3406

[dangoor]

I'm going to give @WebsiteDeveloper's fix a try and see if it works for me.

[njx]

Oh, I missed the IRC chat about this--sounds like that wasn't the problem.

[dangoor]

Nope, just changing the event being listened to does not fix the problem for me.

[njx]

Sounds like rolling back #3406 is the right thing to do for now then given that we're at the end of the sprint. @RaymondLim could you review @dangoor's rollback PR (#5415)?

[WebsiteDeveloper]

Alright, i agree. I will try to integrate the changes cleanly at the beginning of the next sprint.

[dangoor]

@WebsiteDeveloper since it appears that I'm the only one that can consistently reproduce the issue, I'll be happy to test out the next iteration of the change.

[RaymondLim]

FBNC to @dangoor since I can't reproduce it even before the reversion of #3406.

[jbalsas]

Hi guys!

I was also hitting this and able to reproduce by switching editors.

Actually, and in case you're still on time, the issue is the var declarations in https://github.com/adobe/brackets/pull/3406/files#diff-170376f6658e67e879f2acf1e2e2209bR622

Since everytime we enter on activeEditorChangeHandler we create the handlers, then the handlers passed to the on and off methods never match, so those listeners are never being removed and we end up executing the keyEventHandler multiple times. (As many as times we've ended up in our current editor).

Moving the var declarations outside activeEditorHandler seems to fix it for me :)

[peterflynn]

@jbalsas Good catch! It seems like one could even just rewrite the signatures of handleChange() and handleKeyEvent() so they can be passed directly as the listeners -- those wrapper functions are unneeded.

[njx]

Welcome back @jbalsas, we missed you!

[jbalsas]

Thanks! Feels good to be back! 😉

[dangoor]

@RaymondLim confirmed that reverting this change fixed it.

@jbalsas thanks for spotting the fix!

[jbalsas]

I'm not entirely sure, but I have the feeling that I'm still occasionally hitting this, so maybe there's some other condition that's also leaking some listener... I'll try to see if I can pin it down more precisely.