saveNow() now requires a callback

the current code was freezing when clicking on 'cards' in the
browser - it looks like like the javascript callback was never
being called despite calling processEvents(). so we need to
refactor the code to call saveNow() with a callback that does the
subsequent processing.

a lot of the browser code was implicitly calling saveNow() via
beginReset(), so we've had to change all that code to save
immediately before it begins any processing. found a probable bug in
the process - it doesn't look like onRowChange() was saving before
overwriting the note, so theoretically edits could be lost if the
user switched to another card very quickly after typing something.

onSearch() has been split into a GUI-activated onSearchActivated()
that takes care of saving, and a lower level search() that refreshes
the current search. it keeps track of the last search via an instance
variable so that it refreshes properly if a user accidentally adds
some characters to their search without activating the search, then
does something like reverse the sort order.

README.addons

@@ -45,6 +45,11 @@ Anki's webviews are now using WebEngine. Of note:
   modified to use this.
 - Javascript is evaluated asynchronously, so if you need the result of a JS
   expression you can use ankiwebview's evalWithCallback().
+- As a result of this asynchronous behaviour, editor.saveNow() now requires a
+  callback. If your add-on performs actions in the browser, you likely need to
+  call editor.saveNow() first and then run the rest of your code in the callback.
+  Calls to .onSearch() will need to be changed to .search()/.onSearchActivated()
+  as well. See the browser's .deleteNotes() for an example.
 - You can now debug the webviews using an external Chrome instance, by setting
   the env var QTWEBENGINE_REMOTE_DEBUGGING to 8080 prior to starting Anki,
   then surfing to localhost:8080 in Chrome. If you run into issues, try

aqt/addcards.py

@@ -151,7 +151,7 @@ class AddCards(QDialog):
     def editHistory(self, nid):
         browser = aqt.dialogs.open("Browser", self.mw)
         browser.form.searchEdit.lineEdit().setText("nid:%d" % nid)
-        browser.onSearch()
+        browser.onSearchActivated()
 
     def addNote(self, note):
         note.model()['did'] = self.deckChooser.selectedId()
@@ -178,7 +178,9 @@ question on all cards."""), help="AddItems")
         return note
 
     def addCards(self):
-        self.editor.saveNow()
+        self.editor.saveNow(self._addCards)
+
+    def _addCards(self):
         self.editor.saveAddModeVars()
         note = self.editor.note
         note = self.addNote(note)

aqt/browser.py

@@ -115,8 +115,7 @@ class DataModel(QAbstractTableModel):
     # Filtering
     ######################################################################
 
-    def search(self, txt, reset=True):
+    def search(self, txt):
-        if reset:
         self.beginReset()
         t = time.time()
         # the db progress handler may cause a refresh, so we need to zero out
@@ -125,15 +124,14 @@ class DataModel(QAbstractTableModel):
         self.cards = self.col.findCards(txt, order=True)
         #self.browser.mw.pm.profile['fullSearch'])
         #print "fetch cards in %dms" % ((time.time() - t)*1000)
-        if reset:
         self.endReset()
 
     def reset(self):
         self.beginReset()
         self.endReset()
 
+    # caller must have called editor.saveNow() before calling this or .reset()
     def beginReset(self):
-        self.browser.editor.saveNow()
         self.browser.editor.setNote(None, hide=False)
         self.browser.mw.progress.start()
         self.saveSelection()
@@ -147,6 +145,9 @@ class DataModel(QAbstractTableModel):
         self.browser.mw.progress.finish()
 
     def reverse(self):
+        self.browser.editor.saveNow(self._reverse)
+
+    def _reverse(self):
         self.beginReset()
         self.cards.reverse()
         self.endReset()
@@ -351,6 +352,7 @@ class Browser(QMainWindow):
         self.col = self.mw.col
         self.lastFilter = ""
         self._previewWindow = None
+        self._closeEventHasCleanedUp = False
         self.form = aqt.forms.browser.Ui_Dialog()
         self.form.setupUi(self)
         restoreGeom(self, "editor", 0)
@@ -364,17 +366,13 @@ class Browser(QMainWindow):
         self.setupColumns()
         self.setupTable()
         self.setupMenus()
-        self.setupSearch()
         self.setupTree()
         self.setupHeaders()
         self.setupHooks()
         self.setupEditor()
         self.updateFont()
         self.onUndoState(self.mw.form.actionUndo.isEnabled())
-        self.form.searchEdit.setFocus()
+        self.setupSearch()
-        self.form.searchEdit.lineEdit().setText("is:current")
-        self.form.searchEdit.lineEdit().selectAll()
-        self.onSearch()
         self.show()
 
     def setupToolbar(self):
@@ -391,7 +389,6 @@ class Browser(QMainWindow):
             f.actionClose.setVisible(False)
         f.actionReposition.triggered.connect(self.reposition)
         f.actionReschedule.triggered.connect(self.reschedule)
-        f.actionCram.triggered.connect(self.cram)
         f.actionChangeModel.triggered.connect(self.onChangeModel)
         # edit
         f.actionUndo.triggered.connect(self.mw.onUndo)
@@ -458,21 +455,36 @@ class Browser(QMainWindow):
             curmax + 6)
 
     def closeEvent(self, evt):
+        if not self._closeEventHasCleanedUp:
+            if self.editor.note:
+                # ignore event for now to allow us to save
+                self.editor.saveNow(self._closeEventAfterSave)
+                evt.ignore()
+            else:
+                self._closeEventCleanup()
+                evt.accept()
+                self.mw.gcWindow(self)
+        else:
+            evt.accept()
+            self.mw.gcWindow(self)
+
+    def _closeEventAfterSave(self):
+        self._closeEventCleanup()
+        self.close()
+
+    def _closeEventCleanup(self):
+        self.editor.setNote(None)
         saveSplitter(self.form.splitter_2, "editor2")
         saveSplitter(self.form.splitter, "editor3")
-        self.editor.saveNow()
-        self.editor.setNote(None)
         saveGeom(self, "editor")
         saveState(self, "editor")
         saveHeader(self.form.tableView.horizontalHeader(), "editor")
         self.col.conf['activeCols'] = self.model.activeCols
         self.col.setMod()
-        self.hide()
-        aqt.dialogs.close("Browser")
         self.teardownHooks()
         self.mw.maybeReset()
-        self.mw.gcWindow(self)
+        aqt.dialogs.close("Browser")
-        evt.accept()
+        self._closeEventHasCleanedUp = True
 
     def canClose(self):
         return True
@@ -510,19 +522,31 @@ class Browser(QMainWindow):
     ######################################################################
 
     def setupSearch(self):
-        self.filterTimer = None
         self.form.searchEdit.setLineEdit(FavouritesLineEdit(self.mw, self))
-        self.form.searchButton.clicked.connect(self.onSearch)
+        self.form.searchButton.clicked.connect(self.onSearchActivated)
-        self.form.searchEdit.lineEdit().returnPressed.connect(self.onSearch)
+        self.form.searchEdit.lineEdit().returnPressed.connect(self.onSearchActivated)
         self.form.searchEdit.setCompleter(None)
         self.form.searchEdit.addItems(self.mw.pm.profile['searchHistory'])
+        self._searchPrompt = _("<type here to search; hit enter to show current deck>")
+        self._lastSearchTxt = "is:current"
+        self.search()
+        # then replace text for easily showing the deck
+        self.form.searchEdit.lineEdit().setText(self._searchPrompt)
+        self.form.searchEdit.lineEdit().selectAll()
+        self.form.searchEdit.setFocus()
 
-    def onSearch(self, reset=True):
+    # search triggered by user
-        "Careful: if reset is true, the current note is saved."
+    def onSearchActivated(self):
+        self.editor.saveNow(self._onSearchActivated)
+
+    def _onSearchActivated(self):
+        # convert guide text before we save history
+        if self.form.searchEdit.lineEdit().text() == self._searchPrompt:
+            self.form.searchEdit.lineEdit().setText("deck:current ")
+
+        # update history
         txt = str(self.form.searchEdit.lineEdit().text()).strip()
-        prompt = _("<type here to search; hit enter to show current deck>")
         sh = self.mw.pm.profile['searchHistory']
-        # update search history
         if txt in sh:
             sh.remove(txt)
         sh.insert(0, txt)
@@ -530,30 +554,25 @@ class Browser(QMainWindow):
         self.form.searchEdit.clear()
         self.form.searchEdit.addItems(sh)
         self.mw.pm.profile['searchHistory'] = sh
-        if self.mw.state == "review" and "is:current" in txt:
+
-            # search for current card, but set search to easily display whole
+        # keep track of search string so that we reuse identical search when
-            # deck
+        # refreshing, rather than whatever is currently in the search field
-            if reset:
+        self._lastSearchTxt = txt
-                self.model.beginReset()
+        self.search()
-                self.model.focusedCard = self.mw.reviewer.card.id
+
-            self.model.search("nid:%d"%self.mw.reviewer.card.nid, False)
+    # search triggered programmatically. caller must have saved note first.
-            if reset:
+    def search(self):
-                self.model.endReset()
+        if "is:current" in self._lastSearchTxt:
-            self.form.searchEdit.lineEdit().setText(prompt)
+            # show current card if there is one
-            self.form.searchEdit.lineEdit().selectAll()
+            c = self.mw.reviewer.card
-            return
+            nid = c and c.nid or 0
-        elif "is:current" in txt:
+            self.model.search("nid:%d"%nid)
-            self.form.searchEdit.lineEdit().setText(prompt)
+        else:
-            self.form.searchEdit.lineEdit().selectAll()
+            self.model.search(self._lastSearchTxt)
-        elif txt == prompt:
+
-            self.form.searchEdit.lineEdit().setText("deck:current ")
-            txt = "deck:current "
-        self.model.search(txt, reset)
         if not self.model.cards:
             # no row change will fire
-            self.onRowChanged(None, None)
+            self._onRowChanged(None, None)
-        elif self.mw.state == "review":
-            self.focusCid(self.mw.reviewer.card.id)
 
     def updateTitle(self):
         selected = len(self.form.tableView.selectionModel().selectedRows())
@@ -568,7 +587,7 @@ class Browser(QMainWindow):
 
     def onReset(self):
         self.editor.setNote(None)
-        self.onSearch()
+        self.search()
 
     # Table view & editor
     ######################################################################
@@ -588,6 +607,9 @@ class Browser(QMainWindow):
 
     def onRowChanged(self, current, previous):
         "Update current note and hide/show editor."
+        self.editor.saveNow(lambda: self._onRowChanged(current, previous))
+
+    def _onRowChanged(self, current, previous):
         update = self.updateTitle()
         show = self.model.cards and update == 1
         self.form.splitter.widget(1).setVisible(not not show)
@@ -636,14 +658,16 @@ class Browser(QMainWindow):
         hh.sectionMoved.connect(self.onColumnMoved)
 
     def onSortChanged(self, idx, ord):
+        self.editor.saveNow(lambda: self._onSortChanged(idx, ord))
+
+    def _onSortChanged(self, idx, ord):
         type = self.model.activeCols[idx]
         noSort = ("question", "answer", "template", "deck", "note", "noteTags")
         if type in noSort:
             if type == "template":
-                # fixme: change to 'card:1' to be clearer in future dev round
                 showInfo(_("""\
 This column can't be sorted on, but you can search for individual card types, \
-such as 'card:Card 1'."""))
+such as 'card:1'."""))
             elif type == "deck":
                 showInfo(_("""\
 This column can't be sorted on, but you can search for specific decks \
@@ -658,7 +682,7 @@ by clicking on one on the left."""))
             if type == "noteFld":
                 ord = not ord
             self.col.conf['sortBackwards'] = ord
-            self.onSearch()
+            self.search()
         else:
             if self.col.conf['sortBackwards'] != ord:
                 self.col.conf['sortBackwards'] = ord
@@ -692,6 +716,9 @@ by clicking on one on the left."""))
         m.exec_(gpos)
 
     def toggleField(self, type):
+        self.editor.saveNow(lambda: self._toggleField(type))
+
+    def _toggleField(self, type):
         self.model.beginReset()
         if type in self.model.activeCols:
             if len(self.model.activeCols) < 2:
@@ -778,15 +805,14 @@ by clicking on one on the left."""))
             txt = "-"+txt
         if self.mw.app.keyboardModifiers() & Qt.ControlModifier:
             cur = str(self.form.searchEdit.lineEdit().text())
-            if cur and cur != \
+            if cur and cur != self._searchPrompt:
-                    _("<type here to search; hit enter to show current deck>"):
                         txt = cur + " " + txt
         elif self.mw.app.keyboardModifiers() & Qt.ShiftModifier:
             cur = str(self.form.searchEdit.lineEdit().text())
             if cur:
                 txt = cur + " or " + txt
         self.form.searchEdit.lineEdit().setText(txt)
-        self.onSearch()
+        self.onSearchActivated()
 
     def _systemTagTree(self, root):
         tags = (
@@ -975,15 +1001,13 @@ where id in %s""" % ids2str(sf))
     ######################################################################
 
     def onChangeModel(self):
+        self.editor.saveNow(self._onChangeModel)
+
+    def _onChangeModel(self):
         nids = self.oneModelNotes()
         if nids:
             ChangeModel(self, nids)
 
-    def cram(self):
-        return showInfo("not yet implemented")
-        self.close()
-        self.mw.onCram(self.selectedCards())
-
     # Preview
     ######################################################################
 
@@ -1102,6 +1126,9 @@ where id in %s""" % ids2str(sf))
     ######################################################################
 
     def deleteNotes(self):
+        self.editor.saveNow(self._deleteNotes)
+
+    def _deleteNotes(self):
         nids = self.selectedNotes()
         if not nids:
             return
@@ -1122,7 +1149,7 @@ where id in %s""" % ids2str(sf))
             # last selection at top; place one above topmost selection
             newRow = min(selectedRows) - 1
         self.col.remNotes(nids)
-        self.onSearch(reset=False)
+        self.search()
         if len(self.model.cards):
             newRow = min(newRow, len(self.model.cards) - 1)
             newRow = max(newRow, 0)
@@ -1135,6 +1162,9 @@ where id in %s""" % ids2str(sf))
     ######################################################################
 
     def setDeck(self):
+        self.editor.saveNow(self._setDeck)
+
+    def _setDeck(self):
         from aqt.studydeck import StudyDeck
         cids = self.selectedCards()
         if not cids:
@@ -1171,6 +1201,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def addTags(self, tags=None, label=None, prompt=None, func=None):
+        self.editor.saveNow(lambda: self._addTags(tags, label, prompt, func))
+
+    def _addTags(self, tags, label, prompt, func):
         if prompt is None:
             prompt = _("Enter tags to add:")
         if tags is None:
@@ -1202,11 +1235,11 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     def isSuspended(self):
         return not not (self.card and self.card.queue == -1)
 
-    def onSuspend(self, sus=None):
+    def onSuspend(self):
-        if sus is None:
+        self.editor.saveNow(self._onSuspend)
+
+    def _onSuspend(self):
         sus = not self.isSuspended()
-        # focus lost hook may not have chance to fire
-        self.editor.saveNow()
         c = self.selectedCards()
         if sus:
             self.col.sched.suspendCards(c)
@@ -1230,6 +1263,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def reposition(self):
+        self.editor.saveNow(self._reposition)
+
+    def _reposition(self):
         cids = self.selectedCards()
         cids2 = self.col.db.list(
             "select id from cards where type = 0 and id in " + ids2str(cids))
@@ -1253,7 +1289,7 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
         self.col.sched.sortCards(
             cids, start=frm.start.value(), step=frm.step.value(),
             shuffle=frm.randomize.isChecked(), shift=frm.shift.isChecked())
-        self.onSearch(reset=False)
+        self.search()
         self.mw.requireReset()
         self.model.endReset()
 
@@ -1261,6 +1297,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def reschedule(self):
+        self.editor.saveNow(self._reschedule)
+
+    def _reschedule(self):
         d = QDialog(self)
         d.setWindowModality(Qt.WindowModal)
         frm = aqt.forms.reschedule.Ui_Dialog()
@@ -1277,7 +1316,7 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
             fmax = max(fmin, fmax)
             self.col.sched.reschedCards(
                 self.selectedCards(), fmin, fmax)
-        self.onSearch(reset=False)
+        self.search()
         self.mw.requireReset()
         self.model.endReset()
 
@@ -1285,12 +1324,17 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def selectNotes(self):
+        self.editor.saveNow(self._selectNotes)
+
+    def _selectNotes(self):
         nids = self.selectedNotes()
-        self.form.searchEdit.lineEdit().setText("nid:"+",".join([str(x) for x in nids]))
+        # bypass search history
+        self._lastSearchTxt = "nid:"+",".join([str(x) for x in nids])
+        self.form.searchEdit.lineEdit().setText(self._lastSearchTxt)
         # clear the selection so we don't waste energy preserving it
         tv = self.form.tableView
         tv.selectionModel().clear()
-        self.onSearch()
+        self.search()
         tv.selectAll()
 
     def invertSelection(self):
@@ -1327,6 +1371,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def onFindReplace(self):
+        self.editor.saveNow(self._onFindReplace)
+
+    def _onFindReplace(self):
         sf = self.selectedNotes()
         if not sf:
             return
@@ -1361,7 +1408,7 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
             showInfo(_("Invalid regular expression."), parent=self)
             return
         else:
-            self.onSearch()
+            self.search()
             self.mw.requireReset()
         finally:
             self.model.endReset()
@@ -1380,6 +1427,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     ######################################################################
 
     def onFindDupes(self):
+        self.editor.saveNow(self._onFindDupes)
+
+    def _onFindDupes(self):
         d = QDialog(self)
         self.mw.setupDialogGC(d)
         frm = aqt.forms.finddupes.Ui_Dialog()
@@ -1441,7 +1491,9 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
 
     def dupeLinkClicked(self, link):
         self.form.searchEdit.lineEdit().setText(link)
-        self.onSearch()
+        # manually, because we've already saved
+        self._lastSearchTxt = link
+        self.search()
         self.onNote()
 
     # Jumping
@@ -1450,7 +1502,6 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
     def _moveCur(self, dir=None, idx=None):
         if not self.model.cards:
             return
-        self.editor.saveNow()
         tv = self.form.tableView
         if idx is None:
             idx = tv.moveCursor(dir, self.mw.app.keyboardModifiers())
@@ -1458,12 +1509,18 @@ update cards set usn=?, mod=?, did=? where id in """ + scids,
         tv.setCurrentIndex(idx)
 
     def onPreviousCard(self):
+        self.editor.saveNow(self._onPreviousCard)
+
+    def _onPreviousCard(self):
         f = self.editor.currentField
         self._moveCur(QAbstractItemView.MoveUp)
         self.editor.web.setFocus()
         self.editor.web.eval("focusField(%d)" % f)
 
     def onNextCard(self):
+        self.editor.saveNow(self._onNextCard)
+
+    def _onNextCard(self):
         f = self.editor.currentField
         self._moveCur(QAbstractItemView.MoveDown)
         self.editor.web.setFocus()
@@ -1675,7 +1732,7 @@ Are you sure you want to continue?""")):
         b.model.beginReset()
         mm = b.mw.col.models
         mm.change(self.oldModel, self.nids, self.targetModel, fmap, cmap)
-        b.onSearch(reset=False)
+        b.search()
         b.model.endReset()
         b.mw.progress.finish()
         b.mw.reset()

aqt/editcurrent.py

@@ -51,8 +51,10 @@ class EditCurrent(QDialog):
         self.editor.setNote(n)
 
     def onSave(self):
+        self.editor.saveNow(self._onSave)
+
+    def _onSave(self):
         remHook("reset", self.onReset)
-        self.editor.saveNow()
         r = self.mw.reviewer
         try:
             r.card.load()

aqt/editor.py

@@ -80,6 +80,7 @@ function setFGButton(col) {
 };
 
 function saveNow() {
+    clearChangeTimer();
     if (currentField) {
         currentField.blur();
     }
@@ -453,13 +454,17 @@ class Editor(object):
     #     _("Record audio (F5)")
 
     def onFields(self):
+        self.saveNow(self._onFields)
+
+    def _onFields(self):
         from aqt.fields import FieldDialog
-        self.saveNow()
         FieldDialog(self.mw, self.note, parent=self.parentWindow)
 
     def onCardLayout(self):
+        self.saveNow(self._onCardLayout)
+
+    def _onCardLayout(self):
         from aqt.clayout import CardLayout
-        self.saveNow()
         if self.card:
             ord = self.card.ord
         else:
@@ -581,20 +586,13 @@ class Editor(object):
         return [(f['font'], f['size'], f['rtl'])
                 for f in self.note.model()['flds']]
 
-    def saveNow(self):
+    def saveNow(self, callback):
-        "Must call this before adding cards, closing dialog, etc."
+        "Save unsaved edits then call callback()."
         if not self.note:
+            callback()
             return
         self.saveTags()
-        # move focus out of fields and save tags
+        self.web.evalWithCallback("saveNow()", lambda res: callback())
-        self._saveNowWaiting = True
-        self.web.evalWithCallback("saveNow()", self._saveNowDone)
-        while self._saveNowWaiting:
-            # and process events so any focus-lost hooks fire
-            self.mw.app.processEvents()
-
-    def _saveNowDone(self, res):
-        self._saveNowWaiting = False
 
     def checkValid(self):
         cols = []
@@ -615,7 +613,7 @@ class Editor(object):
         browser.form.searchEdit.lineEdit().setText(
             '"dupe:%s,%s"' % (self.note.model()['id'],
                               contents))
-        browser.onSearch()
+        browser.onSearchActivated()
 
     def fieldsAreBlank(self):
         if not self.note:
@@ -630,7 +628,9 @@ class Editor(object):
     ######################################################################
 
     def onHtmlEdit(self):
-        self.saveNow()
+        self.saveNow(self._onHtmlEdit)
+
+    def _onHtmlEdit(self):
         d = QDialog(self.widget)
         form = aqt.forms.edithtml.Ui_Dialog()
         form.setupUi(d)