Merge pull request adobe#4173 from adobe/nj/issue-4172

Ignore key and close code hint session if code hint list is invisible

Author: gruehle

src/editor/CodeHintList.js

@@ -339,6 +339,12 @@ define(function (require, exports, module) {
 
             return itemsPerPage;
         }
+        
+        // If we're no longer visible, skip handling the key and end the session.
+        if (!this.isOpen()) {
+            this.handleClose();
+            return false;
+        }
 
         // (page) up, (page) down, enter and tab key are handled by the list
         if (event.type === "keydown" && this.isHandlingKeyCode(event.keyCode)) {
@@ -353,8 +359,7 @@ define(function (require, exports, module) {
 
                 // Let the event bubble.
                 return false;
-            }
-            else if (keyCode === KeyEvent.DOM_VK_UP) {
+            } else if (keyCode === KeyEvent.DOM_VK_UP) {
                 _rotateSelection.call(this, -1);
             } else if (keyCode === KeyEvent.DOM_VK_DOWN) {
                 _rotateSelection.call(this, 1);
@@ -470,8 +475,9 @@ define(function (require, exports, module) {
         // TODO: Due to #1381, this won't get called if the user clicks out of
         // the code hint menu. That's (sort of) okay right now since it doesn't
         // really matter if a single old invisible code hint list is lying 
-        // around (it'll get closed the next time the user pops up a code 
-        // hint). Once #1381 is fixed this issue should go away.
+        // around (it will ignore keydown events, and it'll get closed the next 
+        // time the user pops up a code hint). Once #1381 is fixed this issue 
+        // should go away.
         this.handleClose = callback;
     };
 

test/spec/CodeHint-test.js

@@ -34,7 +34,8 @@ define(function (require, exports, module) {
         Commands        = require("command/Commands"),
         EditorManager,      // loaded from brackets.test
         CommandManager,
-        CodeHintManager;
+        CodeHintManager,
+        KeyBindingManager;
 
     var testPath = SpecRunnerUtils.getTestPath("/spec/CodeHint-test-files"),
         testWindow,
@@ -75,6 +76,7 @@ define(function (require, exports, module) {
                 CodeHintManager     = testWindow.brackets.test.CodeHintManager;
                 EditorManager       = testWindow.brackets.test.EditorManager;
                 CommandManager      = testWindow.brackets.test.CommandManager;
+                KeyBindingManager   = testWindow.brackets.test.KeyBindingManager;
             });
         });
 
@@ -267,6 +269,48 @@ define(function (require, exports, module) {
                     expectNoHints();
                 });
             });
+            
+            it("should stop handling keydowns if closed by a click outside", function () {
+                var editor,
+                    pos = {line: 3, ch: 1};
+
+                // minimal markup with an open '<' before IP
+                // Note: line for pos is 0-based and editor lines numbers are 1-based
+                initCodeHintTest("test1.html", pos);
+
+                runs(function () {
+                    editor = EditorManager.getCurrentFullEditor();
+                    expect(editor).toBeTruthy();
+                    
+                    editor.document.replaceRange("di", pos);
+                    invokeCodeHints();
+                    
+                    // verify list is open
+                    expectSomeHints();
+                    
+                    // get the document text and make sure it doesn't change if we
+                    // click outside and then keydown
+                    var text = editor.document.getText();
+                    
+                    testWindow.$("body").click();
+                    KeyBindingManager._handleKeyEvent({
+                        keyCode: KeyEvent.DOM_VK_ENTER,
+                        stopImmediatePropagation: function () { },
+                        stopPropagation: function () { },
+                        preventDefault: function () { }
+                    });
+                    
+                    // Verify that after the keydown, the session is closed 
+                    // (not just the hint popup). Because of #1381, we don't
+                    // actually have a way to close the session as soon as the
+                    // popup is dismissed by Bootstrap, so we do so on the next
+                    // keydown. Eventually, once that's fixed, we should be able
+                    // to move this expectNoHints() up after the click.
+                    expectNoHints();
+                    expect(editor.document.getText()).toEqual(text);
+                });
+                
+            });
         });
     });
 });