refactor editor focus handling

this fixes a bug where navigating to the next/previous card using
shortcut keys resulted in the first field being clobbered

- get rid of the stealFocus option in favour of explicitly passing
focusTo to setNote()
- setFields() is no longer responsible for setting focus
- add focusTo var to the browser so that the row changed hook can
restore focus when navigating to next/previous card
- fix the row changed hook being called twice
- the blur event now includes the field number instead of relying on the
 editor to have the correct currentField
- the current field is set to null on blur
- use deferred js and a callback rather than keeping track of when we
were loaded
- add shift+tab shortcut to go to previous field

aqt/addcards.py

@@ -34,7 +34,8 @@ class AddCards(QDialog):
         addHook('currentModelChanged', self.onModelChange)
         addCloseShortcut(self)
         self.show()
-        self.setupNewNote()
+        n = self.mw.col.newNote()
+        self.setAndFocusNote(n)
 
     def setupEditor(self):
         self.editor = aqt.editor.Editor(
@@ -79,15 +80,12 @@ class AddCards(QDialog):
         b.setEnabled(False)
         self.historyButton = b
 
-    def setupNewNote(self, set=True):
+    def setAndFocusNote(self, note):
-        f = self.mw.col.newNote()
+        self.editor.setNote(note, focusTo=0)
-        if set:
-            self.editor.setNote(f, focus=True)
-        return f
 
     def onModelChange(self):
         oldNote = self.editor.note
-        note = self.setupNewNote(set=False)
+        note = self.mw.col.newNote()
         if oldNote:
             oldFields = list(oldNote.keys())
             newFields = list(note.keys())
@@ -107,12 +105,11 @@ class AddCards(QDialog):
                     except IndexError:
                         pass
             self.removeTempNote(oldNote)
-        self.editor.currentField = 0
+        self.editor.setNote(note)
-        self.editor.setNote(note, focus=True)
 
     def onReset(self, model=None, keep=False):
         oldNote = self.editor.note
-        note = self.setupNewNote(set=False)
+        note = self.mw.col.newNote()
         flds = note.model()['flds']
         # copy fields from old note
         if oldNote:
@@ -126,8 +123,7 @@ class AddCards(QDialog):
                         note.fields[n] = ""
                 except IndexError:
                     break
-        self.editor.currentField = 0
+        self.setAndFocusNote(note)
-        self.editor.setNote(note, focus=True)
 
     def removeTempNote(self, note):
         if not note or not note.id:

aqt/browser.py

@@ -359,6 +359,7 @@ class Browser(QMainWindow):
         self.col = self.mw.col
         self.forceClose = False
         self.lastFilter = ""
+        self.focusTo = None
         self._previewWindow = None
         self._closeEventHasCleanedUp = False
         self.form = aqt.forms.browser.Ui_Dialog()
@@ -611,7 +612,6 @@ class Browser(QMainWindow):
     def setupEditor(self):
         self.editor = aqt.editor.Editor(
             self.mw, self.form.fieldsArea, self)
-        self.editor.stealFocus = False
 
     def onRowChanged(self, current, previous):
         "Update current note and hide/show editor."
@@ -627,7 +627,8 @@ class Browser(QMainWindow):
         else:
             self.card = self.model.getCard(
                 self.form.tableView.selectionModel().currentIndex())
-            self.editor.setNote(self.card.note(reload=True))
+            self.editor.setNote(self.card.note(reload=True), focusTo=self.focusTo)
+            self.focusTo = None
             self.editor.card = self.card
             self.singleCard = True
         self._renderPreview(True)
@@ -1517,34 +1518,25 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
         tv = self.form.tableView
         if idx is None:
             idx = tv.moveCursor(dir, self.mw.app.keyboardModifiers())
-        tv.selectionModel().clear()
+        tv.selectionModel().setCurrentIndex(
-        tv.setCurrentIndex(idx)
+            idx,
+            QItemSelectionModel.Clear|
+            QItemSelectionModel.Select|
+            QItemSelectionModel.Rows)
 
     def onPreviousCard(self):
+        self.focusTo = self.editor.currentField
         self.editor.saveNow(self._onPreviousCard)
 
     def _onPreviousCard(self):
-        tagfocus = self.editor.tags.hasFocus()
-        f = self.editor.currentField
         self._moveCur(QAbstractItemView.MoveUp)
-        if tagfocus:
-            self.editor.tags.setFocus()
-            return
-        self.editor.web.setFocus()
-        self.editor.web.eval("focusField(%d)" % f)
 
     def onNextCard(self):
+        self.focusTo = self.editor.currentField
         self.editor.saveNow(self._onNextCard)
 
     def _onNextCard(self):
-        tagfocus = self.editor.tags.hasFocus()
-        f = self.editor.currentField
         self._moveCur(QAbstractItemView.MoveDown)
-        if tagfocus:
-            self.editor.tags.setFocus()
-            return
-        self.editor.web.setFocus()
-        self.editor.web.eval("focusField(%d)" % f)
 
     def onFirstCard(self):
         sm = self.form.tableView.selectionModel()
@@ -1574,7 +1566,6 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
         self.form.searchEdit.lineEdit().selectAll()
 
     def onNote(self):
-        self.editor.focus()
         self.editor.web.setFocus()
         self.editor.web.eval("focusField(0);")
 

aqt/editcurrent.py

@@ -26,7 +26,7 @@ class EditCurrent(QDialog):
         self.form.buttonBox.button(QDialogButtonBox.Close).setShortcut(
                 QKeySequence("Ctrl+Return"))
         self.editor = aqt.editor.Editor(self.mw, self.form.fieldsArea, self)
-        self.editor.setNote(self.mw.reviewer.card.note())
+        self.editor.setNote(self.mw.reviewer.card.note(), focusTo=0)
         restoreGeom(self, "editcurrent")
         addHook("reset", self.onReset)
         self.mw.requireReset()

aqt/editor.py

@@ -42,10 +42,8 @@ class Editor:
         self.widget = widget
         self.parentWindow = parentWindow
         self.note = None
-        self.stealFocus = True
         self.addMode = addMode
-        self._loaded = False
+        self.currentField = None
-        self.currentField = 0
         # current card, for card layout
         self.card = None
         self.setupOuter()
@@ -69,7 +67,6 @@ class Editor:
         self.web.allowDrops = True
         self.web.onBridgeCmd = self.onBridgeCmd
         self.outerLayout.addWidget(self.web, 1)
-        self.web.onLoadFinished = self._loadFinished
 
         righttopbtns = list()
         righttopbtns.append(self._addButton('text_bold', 'bold', "Bold text (Ctrl+B)", id='bold'))
@@ -158,7 +155,7 @@ class Editor:
             ("Ctrl+T, E", self.insertLatexEqn),
             ("Ctrl+T, M", self.insertLatexMathEnv),
             ("Ctrl+Shift+X", self.onHtmlEdit),
-            ("Ctrl+Shift+T", lambda: self.tags.setFocus),
+            ("Ctrl+Shift+T", self.onFocusTags)
         ]
         runFilter("setupEditorShortcuts", cuts)
         for keys, fn in cuts:
@@ -194,7 +191,8 @@ class Editor:
             return
         # focus lost or key/button pressed?
         if cmd.startswith("blur") or cmd.startswith("key"):
-            (type, txt) = cmd.split(":", 1)
+            (type, ord, txt) = cmd.split(":", 2)
+            ord = int(ord)
             txt = urllib.parse.unquote(txt)
             txt = unicodedata.normalize("NFC", txt)
             txt = self.mungeHTML(txt)
@@ -202,22 +200,18 @@ class Editor:
             txt = txt.replace("\x00", "")
             # reverse the url quoting we added to get images to display
             txt = self.mw.col.media.escapeImages(txt, unescape=True)
-            self.note.fields[self.currentField] = txt
+            self.note.fields[ord] = txt
             if not self.addMode:
                 self.note.flush()
                 self.mw.requireReset()
             if type == "blur":
+                self.currentField = None
                 # run any filters
                 if runFilter(
-                    "editFocusLost", False, self.note, self.currentField):
+                    "editFocusLost", False, self.note, ord):
-                    # something updated the note; schedule reload
+                    # something updated the note; update it after a subsequent focus
-                    def onUpdate():
+                    # event has had time to fire
-                        if not self.note:
+                    self.mw.progress.timer(100, self.loadNote, False)
-                            return
-                        self.stealFocus = True
-                        self.loadNote()
-                        self.checkValid()
-                    self.mw.progress.timer(100, onUpdate, False)
                 else:
                     self.checkValid()
             else:
@@ -241,58 +235,42 @@ class Editor:
     # Setting/unsetting the current note
     ######################################################################
 
-    def _loadFinished(self):
+    def setNote(self, note, hide=True, focusTo=None):
-        self._loaded = True
-
-        # setup colour button
-        self.setupForegroundButton()
-
-        if self.note:
-            self.loadNote()
-
-    def setNote(self, note, hide=True, focus=False):
         "Make NOTE the current note."
         self.note = note
-        self.currentField = 0
+        self.currentField = None
-        if focus:
-            self.stealFocus = True
         if self.note:
-            self.loadNote()
+            self.loadNote(focusTo=focusTo)
         else:
             self.hideCompleters()
             if hide:
                 self.widget.hide()
 
-    def loadNote(self):
+    def loadNote(self, focusTo=None):
         if not self.note:
             return
-        if self.stealFocus:
+
-            field = self.currentField
-        else:
-            field = -1
-        if not self._loaded:
-            # will be loaded when page is ready
-            return
         data = []
         for fld, val in list(self.note.items()):
             data.append((fld, self.mw.col.media.escapeImages(val)))
-        self.web.eval("setFields(%s, %d, %s);" % (
-            json.dumps(data), field, json.dumps(self.prewrapMode())))
-        self.web.eval("setFonts(%s);" % (
-            json.dumps(self.fonts())))
-        self.checkValid()
-        self.updateTags()
         self.widget.show()
-        if self.stealFocus:
+        self.updateTags()
-            self.web.setFocus()
+
-            self.stealFocus = False
+        def oncallback(arg):
+            if not self.note:
+                return
+            self.setupForegroundButton()
+            self.checkValid()
+            runHook("loadNote", self)
+
+        self.web.evalWithCallback("setFields(%s, %s); setFonts(%s); focusField(%s)" % (
+            json.dumps(data), json.dumps(self.prewrapMode()),
+            json.dumps(self.fonts()), json.dumps(focusTo)),
+                                  oncallback)
 
     def prewrapMode(self):
         return self.note.model().get('prewrap', False)
 
-    def focus(self):
-        self.web.setFocus()
-
     def fonts(self):
         return [(f['font'], f['size'], f['rtl'])
                 for f in self.note.model()['flds']]
@@ -339,14 +317,15 @@ class Editor:
     ######################################################################
 
     def onHtmlEdit(self):
-        self.saveNow(self._onHtmlEdit)
+        field = self.currentField
+        self.saveNow(lambda: self._onHtmlEdit(field))
 
-    def _onHtmlEdit(self):
+    def _onHtmlEdit(self, field):
         d = QDialog(self.widget)
         form = aqt.forms.edithtml.Ui_Dialog()
         form.setupUi(d)
         form.buttonBox.helpRequested.connect(lambda: openHelp("editor"))
-        form.textEdit.setPlainText(self.note.fields[self.currentField])
+        form.textEdit.setPlainText(self.note.fields[field])
         form.textEdit.moveCursor(QTextCursor.End)
         d.exec_()
         html = form.textEdit.toPlainText()
@@ -355,11 +334,8 @@ class Editor:
         with warnings.catch_warnings() as w:
             warnings.simplefilter('ignore', UserWarning)
             html = str(BeautifulSoup(html, "html.parser"))
-        self.note.fields[self.currentField] = html
+        self.note.fields[field] = html
-        self.loadNote()
+        self.loadNote(focusTo=field)
-        # focus field so it's saved
-        self.web.setFocus()
-        self.web.eval("focusField(%d);" % self.currentField)
 
     # Tag handling
     ######################################################################
@@ -408,6 +384,9 @@ class Editor:
     def hideCompleters(self):
         self.tags.hideCompleter()
 
+    def onFocusTags(self):
+        self.tags.setFocus()
+
     # Format buttons
     ######################################################################
 

web/editor.js

@@ -33,6 +33,11 @@ function onKey() {
         insertNewline();
         return;
     }
+    // shift+tab goes to previous field
+    if (window.event.which === 9 && window.event.shiftKey) {
+        focusPrevious();
+        return;
+    }
     clearChangeTimer();
     changeTimer = setTimeout(function () {
         updateButtonState();
@@ -120,7 +125,7 @@ function clearChangeTimer() {
 
 function onFocus(elem) {
     currentField = elem;
-    pycmd("focus:" + currentField.id.substring(1));
+    pycmd("focus:" + currentFieldOrdinal());
     enableButtons();
     // don't adjust cursor on mouse clicks
     if (mouseDown) {
@@ -145,9 +150,22 @@ function onFocus(elem) {
 }
 
 function focusField(n) {
+    if (n === null) {
+        return;
+    }
     $("#f" + n).focus();
 }
 
+function focusPrevious() {
+    if (!currentField) {
+        return;
+    }
+    var previous = currentFieldOrdinal() - 1;
+    if (previous >= 0) {
+        focusField(previous);
+    }
+}
+
 function onDragOver(elem) {
     // if we focus the target element immediately, the drag&drop turns into a
     // copy, so note it down for later instead
@@ -175,6 +193,7 @@ function caretToEnd() {
 function onBlur() {
     if (currentField) {
         saveField("blur");
+        currentField = null;
     }
     clearChangeTimer();
     disableButtons();
@@ -186,10 +205,14 @@ function saveField(type) {
         return;
     }
     // type is either 'blur' or 'key'
-    pycmd(type + ":" + currentField.innerHTML);
+    pycmd(type + ":" + currentFieldOrdinal() + ":" + currentField.innerHTML);
     clearChangeTimer();
 }
 
+function currentFieldOrdinal() {
+    return currentField.id.substring(1);
+}
+
 function wrappedExceptForWhitespace(text, front, back) {
     var match = text.match(/^(\s*)([^]*?)(\s*)$/);
     return match[1] + front + match[2] + back + match[3];
@@ -234,7 +257,7 @@ function wrap(front, back) {
     }
 }
 
-function setFields(fields, focusTo, prewrap) {
+function setFields(fields, prewrap) {
     var txt = "";
     for (var i = 0; i < fields.length; i++) {
         var n = fields[i][0];
@@ -250,12 +273,6 @@ function setFields(fields, focusTo, prewrap) {
         txt += "</td></tr>";
     }
     $("#fields").html("<table cellpadding=0 width=100%>" + txt + "</table>");
-    if (!focusTo) {
-        focusTo = 0;
-    }
-    if (focusTo >= 0) {
-        $("#f" + focusTo).focus();
-    }
     maybeDisableButtons();
     prewrapMode = prewrap;
     if (prewrap) {