Post in person review

Removed logic for reverse ordered events
Removed almost all of the log statements
Removed list for should unfocus callbacks
Removed all the logic in focus_editor
Only call focus_editor if the keyboard was used to enter edit mode

IPython/html/static/notebook/js/cell.js

@@ -57,11 +57,6 b' var IPython = (function (IPython) {'
         this.cell_type = this.cell_type || null;
         this.code_mirror = null;
 
-        // This is a list of callbacks that are called when a cell's textual 
-        // region is unfocused.  If one of the callbacks returns True, the cell 
-        // unfocus event will be ignored.  Callbacks will be passed no arguments.
-        this.cancel_unfocus_callbacks = [];
-
         this.create_element();
         if (this.element !== null) {
             this.element.data("cell", this);
@@ -80,8 +75,7 b' var IPython = (function (IPython) {'
     
     // FIXME: Workaround CM Bug #332 (Safari segfault on drag)
     // by disabling drag/drop altogether on Safari
-    // https://github.com/marijnh/CodeMirror/issues/332
-    
+    // https://github.com/marijnh/CodeMirror/issues/332    
     if (utils.browser[0] == "Safari") {
         Cell.options_default.cm_config.dragDrop = false;
     }
@@ -89,11 +83,8 b' var IPython = (function (IPython) {'
     Cell.prototype.mergeopt = function(_class, options, overwrite){
         options = options || {};
         overwrite = overwrite || {};
-        return $.extend(true, {}, _class.options_default, options, overwrite)
-
-    }
-
-
+        return $.extend(true, {}, _class.options_default, options, overwrite);
+    };
 
     /**
      * Empty. Subclasses must implement create_element.
@@ -272,10 +263,6 b' var IPython = (function (IPython) {'
      * Check if this cell's unfocus event was legit.
      */
     Cell.prototype.should_cancel_unfocus = function () {
-        // Try user registered callbacks.
-        for (var i=0; i<this.cancel_unfocus_callbacks.length; i++) {
-            if (this.cancel_unfocus_callbacks[i]()) { return true; }
-        }
         return false;
     };
 
@@ -288,6 +275,17 b' var IPython = (function (IPython) {'
     };
 
     /**
+     * Focus the editor area so a user can type
+     *
+     * NOTE: If codemirror is focused via a mouse click event, you don't want to
+     * call this because it will cause a page jump.
+     * @method focus_editor
+     */
+    Cell.prototype.focus_editor = function () {
+        this.code_mirror.focus();
+    };
+
+    /**
      * Refresh codemirror instance
      * @method refresh
      */
@@ -362,7 +360,7 b' var IPython = (function (IPython) {'
         var text = this.code_mirror.getRange({line:0, ch:0}, cursor);
         text = text.replace(/^\n+/, '').replace(/\n+$/, '');
         return text;
-    }
+    };
 
 
     /**
@@ -373,7 +371,7 b' var IPython = (function (IPython) {'
         var cursor = this.code_mirror.getCursor();
         var last_line_num = this.code_mirror.lineCount()-1;
         var last_line_len = this.code_mirror.getLine(last_line_num).length;
-        var end = {line:last_line_num, ch:last_line_len}
+        var end = {line:last_line_num, ch:last_line_len};
         var text = this.code_mirror.getRange(cursor, end);
         text = text.replace(/^\n+/, '').replace(/\n+$/, '');
         return text;
@@ -417,9 +415,10 b' var IPython = (function (IPython) {'
      **/
     Cell.prototype._auto_highlight = function (modes) {
         //Here we handle manually selected modes
-        if( this.user_highlight != undefined &&  this.user_highlight != 'auto' )
+        var mode;
+        if( this.user_highlight !== undefined &&  this.user_highlight != 'auto' )
         {
-            var mode = this.user_highlight;
+            mode = this.user_highlight;
             CodeMirror.autoLoadMode(this.code_mirror, mode);
             this.code_mirror.setOption('mode', mode);
             return;
@@ -427,22 +426,22 b' var IPython = (function (IPython) {'
         var current_mode = this.code_mirror.getOption('mode', mode);
         var first_line = this.code_mirror.getLine(0);
         // loop on every pairs
-        for( var mode in modes) {
-            var regs = modes[mode]['reg'];
+        for(mode in modes) {
+            var regs = modes[mode].reg;
             // only one key every time but regexp can't be keys...
             for(var i=0; i<regs.length; i++) {
                 // here we handle non magic_modes
-                if(first_line.match(regs[i]) != null) {
+                if(first_line.match(regs[i]) !== null) {
                     if(current_mode == mode){
                         return;
                     }
-                    if (mode.search('magic_') != 0) {
+                    if (mode.search('magic_') !== 0) {
                         this.code_mirror.setOption('mode', mode);
                         CodeMirror.autoLoadMode(this.code_mirror, mode);
                         return;
                     }
-                    var open = modes[mode]['open']|| "%%";
-                    var close = modes[mode]['close']|| "%%end";
+                    var open = modes[mode].open || "%%";
+                    var close = modes[mode].close || "%%end";
                     var mmode = mode;
                     mode = mmode.substr(6);
                     if(current_mode == mode){
@@ -469,14 +468,14 b' var IPython = (function (IPython) {'
             }
         }
         // fallback on default
-        var default_mode
+        var default_mode;
         try {
             default_mode = this._options.cm_config.mode;
         } catch(e) {
             default_mode = 'text/plain';
         }
         if( current_mode === default_mode){
-            return
+            return;
         }
         this.code_mirror.setOption('mode', default_mode);
     };

IPython/html/static/notebook/js/codecell.js

@@ -410,9 +410,9 b' var IPython = (function (IPython) {'
         return false;
     };
 
-    CodeCell.prototype.edit_mode = function () {
+    CodeCell.prototype.edit_mode = function (focus_editor) {
         var cont = IPython.Cell.prototype.edit_mode.apply(this);
-        if (this.mode === 'edit') {
+        if (cont && focus_editor) {
             this.focus_editor();
         }
         return cont;
@@ -422,27 +422,10 b' var IPython = (function (IPython) {'
      * Check if this cell's unfocus event was legit.
      */
     CodeCell.prototype.should_cancel_unfocus = function () {
-        // Call base
-        if (IPython.Cell.prototype.should_cancel_unfocus.apply(this)) { return true; }
-
-        // Cancel this unfocus event if the cell completer is open.
-        return (this.completer && this.completer.is_visible());
-    };
-
-    /**
-     * Focus the editor area so a user can type
-     * @method focus_editor
-     */
-    CodeCell.prototype.focus_editor = function () {
-        // Only focus the CM editor if it is not focused already. This prevents 
-        // jumps related to the previous prompt position.  Here we can't use
-        // IPython.utils.is_focused since it uses document.activeElement which
-        // may not be set by the time this is called.  Instead look at the input
-        // element of codemirror directly to see if it is focused.  Use the
-        // jQuery :focus pseudo selector (http://api.jquery.com/focus-selector/)
-        if (!$(this.code_mirror.win).is(':focus')) {
-            this.code_mirror.focus();
-        }
+        // Cancel this unfocus event if the base wants to cancel or the cell 
+        // completer is open.
+        return IPython.Cell.prototype.should_cancel_unfocus.apply(this) ||
+            (this.completer && this.completer.is_visible());
     };
 
     CodeCell.prototype.select_all = function () {

IPython/html/static/notebook/js/keyboardmanager.js

@@ -676,9 +676,8 b' var IPython = (function (IPython) {'
     };
 
 
-
     // Main keyboard manager for the notebook
-
+    
     var KeyboardManager = function () {
         this.mode = 'command';
         this.enabled = true;
@@ -725,51 +724,49 b' var IPython = (function (IPython) {'
     };
 
     KeyboardManager.prototype.edit_mode = function () {
-        console.log('kb edit');
         this.last_mode = this.mode;
         this.mode = 'edit';
     };
 
     KeyboardManager.prototype.command_mode = function () {
-        console.log('kb command');
         this.last_mode = this.mode;
         this.mode = 'command';
     };
 
     KeyboardManager.prototype.enable = function () {
-        console.log('kb enable');
         this.enabled = true;
     };
 
     KeyboardManager.prototype.disable = function () {
-        console.log('kb disable');
         this.enabled = false;
     };
 
     KeyboardManager.prototype.register_events = function (e) {
         var that = this;
         var handle_focus = function () {
-            console.log('kb focus in');
             that.disable();
         };
         var handle_blur = function () {
-            console.log('kb focus out');
             that.enable();
         };
         e.on('focusin', handle_focus);
         e.on('focusout', handle_blur);
         // TODO: Very strange. The focusout event does not seem fire for the 
-        // bootstrap text boxes on FF25&26...
-        e.find('*').blur(handle_blur);
-        e.on('DOMNodeInserted', function () {
-            e.find('*').blur(handle_blur);
+        // bootstrap textboxes on FF25&26...
+        e.find('input').blur(handle_blur);
+        e.on('DOMNodeInserted', function (event) {
+            var target = $(event.target);
+            if (target.is('input')) {
+                target.blur(handle_blur);
+            } else {
+                target.find('input').blur(handle_blur);    
+            }
           });
         // There are times (raw_input) where we remove the element from the DOM before
         // focusout is called. In this case we bind to the remove event of jQueryUI,
         // which gets triggered upon removal, iff it is focused at the time.
         e.on('remove', function () {
             if (IPython.utils.is_focused(e[0])) {
-                console.log('kb remove');
                 that.enable();
             }
         });

IPython/html/static/notebook/js/notebook.js

@@ -55,12 +55,6 b' var IPython = (function (IPython) {'
         this.notebook_name_blacklist_re = /[\/\\:]/;
         this.nbformat = 3; // Increment this when changing the nbformat
         this.nbformat_minor = 0; // Increment this when changing the nbformat
-
-        // This is a list of callbacks that are called when a cell's textual 
-        // region is unfocused.  If one of the callbacks returns True, the cell 
-        // unfocus event will be ignored.  Callbacks will be passed one argument,
-        // the cell instance.
-        this.cancel_unfocus_callbacks = [];
         this.style();
         this.create_elements();
         this.bind_events();
@@ -122,11 +116,11 b' var IPython = (function (IPython) {'
         });
 
         $([IPython.events]).on('focus_text.Cell', function (event, data) {
-            that.handle_cell_text_focus(that.find_cell_index(data.cell));
+            that.handle_cell_text_focus(data.cell);
         });
 
         $([IPython.events]).on('blur_text.Cell', function (event, data) {
-            that.handle_cell_text_blur(that.find_cell_index(data.cell));
+            that.handle_cell_text_blur(data.cell);
         });
 
         $([IPython.events]).on('status_autorestarting.Kernel', function () {
@@ -522,7 +516,6 b' var IPython = (function (IPython) {'
     };
 
     Notebook.prototype.command_mode = function () {
-        console.log('notebook command_mode()');
 
         // Make sure there isn't an edit mode cell lingering around.
         var cell = this.get_cell(this.get_edit_index());
@@ -540,32 +533,23 b' var IPython = (function (IPython) {'
     };
 
     Notebook.prototype.edit_mode = function (index) {
-        console.log('notebook edit_mode()');
 
         // Either use specified index or selected cell's index.
         // Must explictly check for undefined CBool(0) = false.
+        var focus_editor = false;
         if (index===undefined) {
             index = this.get_selected_index();
+            focus_editor = true;
+        } else {
+            this.select(index);
         }
-        var cell = this.get_cell(index);
         // Make sure the cell exists.
+        var cell = this.get_cell(index);
         if (cell === null) { return; }
 
-        // If another cell is currently in edit mode set it to command mode.
-        var edit_index = this.get_edit_index(index);
-        if (edit_index !== null) { // Must explictly check for null CBool(0) = false.
-            var edit_cell = this.get_cell(edit_index);
-            if (edit_cell) {
-                edit_cell.command_mode();
-            }
-        }
-
         // Set the cell to edit mode and notify the keyboard manager if this
         // is a change of mode for the notebook as a whole.
-        if (this.get_selected_index()!==index) {
-            this.select(index);
-        }
-        cell.edit_mode();
+        cell.edit_mode(focus_editor);
         if (this.mode !== 'edit') {
             this.mode = 'edit';
             $([IPython.events]).trigger('edit_mode.Notebook');
@@ -579,30 +563,20 b' var IPython = (function (IPython) {'
         cell.focus_cell();
     };
 
-    Notebook.prototype.handle_cell_text_focus = function (index) {
-        this.edit_mode(index);
+    Notebook.prototype.handle_cell_text_focus = function (cell) {
+        console.log('notebook.handle_cell_text_focus', cell);
+        this.edit_mode(this.find_cell_index(cell));
     };
 
-    Notebook.prototype.handle_cell_text_blur = function (index) {
-        var cell = this.get_cell(index);
-        if (!cell) {return;}
+    Notebook.prototype.handle_cell_text_blur = function (cell) {
+        // In Firefox the focus event is called before the blur event.  In 
+        // other words, two cells elements may be focused at any given time.
+        // This has been witnessed on Win7 x64 w/ FF 25&26.
+        console.log('notebook.handle_cell_text_blur', cell);
 
         // Check if this unfocus event is legit.
         if (!this.should_cancel_unfocus(cell)) {
-            // In Firefox the focus event is called before the blur event.  In 
-            // other words, two cells elements may be focused at any given time.
-            // This has been witnessed on Win7 x64 w/ FF 25.  Here we only put the
-            // entire notebook in command mode iff the cell textbox being blured is
-            // the one that is currently in edit mode.  Otherwise, we assume the
-            // event order has been reversed and we just put this particular cell
-            // in command mode.
-            if (index===this.get_edit_index(index)) {
-                console.log('full command_mode');
-                this.command_mode();
-            } else {
-                console.log('cell command_mode');
-                cell.command_mode();
-            }    
+            this.command_mode();
         }
     };
 
@@ -612,18 +586,9 b' var IPython = (function (IPython) {'
         // If the tooltip is visible, ignore the unfocus.
         var tooltip_visible = IPython.tooltip && IPython.tooltip.is_visible();
         if (tooltip_visible) { return true; }
-        
-        // Try user registered callbacks.
-        for (var i=0; i<this.cancel_unfocus_callbacks.length; i++) {
-            if (this.cancel_unfocus_callbacks[i](cell)) { return true; }
-        }
 
         // Check the cell's should_cancel_unfocus method.
-        if (cell.should_cancel_unfocus !== undefined && cell.should_cancel_unfocus()) {
-            return true;
-        } else {
-            return false;
-        }
+        return (cell.should_cancel_unfocus !== undefined && cell.should_cancel_unfocus());
     };
 
     // Cell movement
@@ -1482,7 +1447,6 b' var IPython = (function (IPython) {'
         var cell = this.get_selected_cell();
         var cell_index = this.find_cell_index(cell);
         
-        console.log('execute cell command_mode');
         cell.execute();
         cell.focus_cell();
         this.command_mode();
@@ -1503,16 +1467,14 b' var IPython = (function (IPython) {'
         // If we are at the end always insert a new cell and return
         if (cell_index === (this.ncells()-1)) {
             this.insert_cell_below('code');
-            this.select(cell_index+1);
-            this.edit_mode();
+            this.edit_mode(cell_index+1);
             this.scroll_to_bottom();
             this.set_dirty(true);
             return;
         }
   
         this.insert_cell_below('code');
-        this.select(cell_index+1);
-        this.edit_mode();
+        this.edit_mode(cell_index+1);
         this.set_dirty(true);
     };
 
@@ -1531,8 +1493,7 b' var IPython = (function (IPython) {'
         // If we are at the end always insert a new cell and return
         if (cell_index === (this.ncells()-1)) {
             this.insert_cell_below('code');
-            this.select(cell_index+1);
-            this.edit_mode();
+            this.edit_mode(cell_index+1);
             this.scroll_to_bottom();
             this.set_dirty(true);
             return;
@@ -2008,8 +1969,7 b' var IPython = (function (IPython) {'
         console.log('load notebook success');
         if (this.ncells() === 0) {
             this.insert_cell_below('code');
-            this.select(0);
-            this.edit_mode();
+            this.edit_mode(0);
         } else {
             this.select(0);
             this.command_mode();
@@ -2369,3 +2329,4 b' var IPython = (function (IPython) {'
     return IPython;
 
 }(IPython));
+

IPython/html/static/notebook/js/textcell.js

@@ -81,7 +81,7 b' var IPython = (function (IPython) {'
         this.celltoolbar = new IPython.CellToolbar(this);
         inner_cell.append(this.celltoolbar.element);
         var input_area = $('<div/>').addClass('text_cell_input border-box-sizing');
-        this.code_mirror = CodeMirror(input_area.get(0), this.cm_config);
+        this.code_mirror = new CodeMirror(input_area.get(0), this.cm_config);
         // The tabindex=-1 makes this div focusable.
         var render_area = $('<div/>').addClass('text_cell_render border-box-sizing').
             addClass('rendered_html').attr('tabindex','-1');
@@ -104,7 +104,7 b' var IPython = (function (IPython) {'
         this.element.dblclick(function () {
             if (that.selected === false) {
                 $([IPython.events]).trigger('select.Cell', {'cell':that});
-            };
+            }
             $([IPython.events]).trigger('edit_mode.Cell', {cell: that});
         });
     };
@@ -145,7 +145,7 b' var IPython = (function (IPython) {'
                 return false;
             } else {
                 return true;
-            };
+            }
         } else if (event.which === key.DOWNARROW && event.type === 'keydown') {
             // If we are not at the bottom, let CM handle the down arrow and
             // prevent the global keydown handler from handling it.
@@ -154,7 +154,7 b' var IPython = (function (IPython) {'
                 return false;
             } else {
                 return true;
-            };
+            }
         } else if (event.which === key.ESC && event.type === 'keydown') {
             if (that.code_mirror.options.keyMap === "vim-insert") {
                 // vim keyMap is active and in insert mode. In this case we leave vim
@@ -179,7 +179,7 b' var IPython = (function (IPython) {'
             if (this.mode === 'edit') {
                 this.code_mirror.refresh();
             }
-        };
+        }
         return cont;
     };
 
@@ -196,7 +196,7 b' var IPython = (function (IPython) {'
                 this.refresh();
             }
 
-        };
+        }
         return cont;
     };
 
@@ -204,14 +204,19 b' var IPython = (function (IPython) {'
         this.render();
     };
 
-    TextCell.prototype.edit_mode = function () {
+    TextCell.prototype.edit_mode = function (focus_editor) {
         var cont = IPython.Cell.prototype.edit_mode.apply(this);
         if (cont) {
-            this.unrender();
-            this.focus_editor();
-        };
+            cont = this.unrender();
+            // Focus the editor if codemirror was just added to the page or the
+            // caller explicitly wants to focus the editor (usally when the 
+            // edit_mode was triggered by something other than a mouse click).
+            if (cont || focus_editor) {
+                this.focus_editor();
+            }
+        }
         return cont;
-    }
+    };
 
     /**
      * setter: {{#crossLink "TextCell/set_text"}}{{/crossLink}}
@@ -261,8 +266,8 b' var IPython = (function (IPython) {'
                 return true;
             } else {
                 return false;
-            };
-        };
+            }
+        }
     };
 
     /**
@@ -278,8 +283,8 b' var IPython = (function (IPython) {'
                 return true;
             } else {
                 return false;
-            };
-        };
+            }
+        }
     };
 
     /**
@@ -332,7 +337,7 b' var IPython = (function (IPython) {'
             mode: 'gfm'
         },
         placeholder: "Type *Markdown* and LaTeX: $\\alpha^2$"
-    }
+    };
 
     MarkdownCell.prototype = new TextCell();
 
@@ -363,8 +368,8 b' var IPython = (function (IPython) {'
             }
             this.element.find('div.text_cell_input').hide();
             this.element.find("div.text_cell_render").show();
-            this.typeset()
-        };
+            this.typeset();
+        }
         return cont;
     };
 
@@ -378,7 +383,7 b' var IPython = (function (IPython) {'
      */
     var RawCell = function (options) {
 
-        options = this.mergeopt(RawCell,options)
+        options = this.mergeopt(RawCell,options);
         TextCell.apply(this, [options]);
         this.cell_type = 'raw';
         // RawCell should always hide its rendered div
@@ -396,7 +401,7 b' var IPython = (function (IPython) {'
     /** @method bind_events **/
     RawCell.prototype.bind_events = function () {
         TextCell.prototype.bind_events.apply(this);
-        var that = this
+        var that = this;
         this.element.focusout(function() {
             that.auto_highlight();
         });
@@ -452,7 +457,7 b' var IPython = (function (IPython) {'
 
     /** @method fromJSON */
     HeadingCell.prototype.fromJSON = function (data) {
-        if (data.level != undefined){
+        if (data.level !== undefined){
             this.level = data.level;
         }
         TextCell.prototype.fromJSON.apply(this, arguments);
@@ -492,7 +497,7 b' var IPython = (function (IPython) {'
         if (this.rendered) {
             this.rendered = false;
             this.render();
-        };
+        }
     };
 
     /** The depth of header cell, based on html (h1 to h6)
@@ -544,7 +549,7 b' var IPython = (function (IPython) {'
             this.element.find('div.text_cell_input').hide();
             this.element.find("div.text_cell_render").show();
 
-        };
+        }
         return cont;
     };