D29148: [KJotsWidget] Don't switch focus to KJotsEdit on dataChanged

Igor Poboiko noreply at phabricator.kde.org
Sat Apr 25 23:44:54 BST 2020


poboiko updated this revision to Diff 81212.
poboiko added a comment.


  Fixed a race condition that could happen due to focusOutEvent
  
  When user changes the document via `KJotsTreeView`, there are several places which react to this change:
  
  1. Slot `KJotsWidget::selectionChanged`, which tries to save the "old" document
  2. Slot `KJotsEdit::selectionChanged`, which tries to `setReadOnly` based on first element of selection
  3. Slots `KJotsWidget::rowsInserted/rowsRemoved`, which change the `KJotsEdit::document()` to the new one
  4. `KJotsEdit::focusOutEvent` triggers, which also tries to save the "old" document.
  
  The `focusOutEvent` could actually come AFTER the selection has changed, but BEFORE the `setDocument` call,
  which leaves `KJotsEdit` in an invalid state: the document() corresponds to one note, while `selection` points to another. 
  Because of that, `savePage` actually overwrites "new" note with the contents of "old" one. I lost several notes because of it.
  
  This is hard to trigger manually, but it's sufficient to do a random combination of following things:
  
  - Select couple of notes from a book using mouse, so `KJotsBrowser` appears
  - Select a single note from a book using mouse
  - Quickly press `Ctrl+PageUp` / `Ctrl+PageDown` multiple times to switch between notes
  
  Instead, I propose the following:
  
  - Don't let `KJotsEdit` interact with `selectionModel`, instead just provide it with a single `QModelIndex` pointing to a document
  
  (so that its state is always consistent)
  
  - This is done via `KJotsEdit::setModelIndex` method, which does all the work in a single place:
    - Saves the old document if necessary
    - Calls `KJotsEdit::setDocument` if necessary
    - Also sets the `QTextCursor` (if it was stored)
    - tries to `setReadOnly` based on `NoteLockAttribute`

REPOSITORY
  R573 KJots

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29148?vs=81083&id=81212

BRANCH
  focus (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29148

AFFECTED FILES
  src/kjotsbrowser.cpp
  src/kjotsbrowser.h
  src/kjotsedit.cpp
  src/kjotsedit.h
  src/kjotswidget.cpp
  src/kjotswidget.h

To: poboiko, dvratil
Cc: kde-pim, fbampaloukas, dcaliste, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200425/b10b3426/attachment.html>


More information about the kde-pim mailing list