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