<table><tr><td style="">poboiko retitled this revision from "[KJotsWidget] Don't switch focus to KJotsEdit on dataChanged" to "[KJotsWidget] Fix several KJotsEdit focus issues (race condition)".<br />poboiko edited the summary of this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-lsl7mlo5axbefrf/">(Show Details)</a>
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D29148">View Revision</a></tr></table><br /><div><strong>CHANGES TO REVISION SUMMARY</strong><div><div style="white-space: pre-wrap; color: #74777D;"><div style="padding: 8px 0;">...</div>with some explicit user action (and we don't want focus to randomly jump unless user exlicitly wants it).<br />
<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">Also, don't use `const_cast` for `QAbstractSelectionModel::model()` and use `non-const` overload instead.<br />
<br />
</span>This fixes following frustrating issue:<div style="padding: 8px 0;">...</div>because autosave is skipped when document is not modified)<span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);"><br />
<br />
This patch also fixes a race condition that could happen due to the `focusOutEvent`. When user changes the document via KJotsTreeView, there are several places which react to this change:<br />
 1. Slot `KJotsWidget::selectionChanged`, which tries to save the "old" document<br />
 2. Slot `KJotsEdit::selectionChanged`, which tries to `setReadOnly` based on first element of selection<br />
 3. Slots `KJotsWidget::rowsInserted/rowsRemoved`, which change the `KJotsEdit::document()` to the new one<br />
 4. `KJotsEdit::focusOutEvent` triggers, which also tries to save the "old" document.<br />
<br />
The `focusOutEvent` could actually come AFTER the selection has changed, but BEFORE the setDocument call,<br />
which leaves `KJotsEdit` in an invalid state: the `document()` corresponds to one note, while selection points to another.<br />
Because of that, `savePage` actually overwrites "new" note with the contents of "old" one. I lost several notes because of it.<br />
<br />
This is hard to trigger manually, but it's sufficient to do a random combination of following things:<br />
 - Select couple of notes from a book using mouse, so KJotsBrowser appears<br />
 - Select a single note from a book using mouse<br />
 - Quickly press Ctrl+PageUp / Ctrl+PageDown multiple times to switch between notes<br />
<br />
Instead, I propose the following:<br />
 - 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)<br />
 - This is done via `KJotsEdit::setModelIndex` method, which does all the work in a single place:<br />
    - Saves the old document if necessary<br />
    - Calls `KJotsEdit::setDocument` if necessary<br />
    - Also sets the `QTextCursor` (if it was stored)<br />
    - tries to `setReadOnly` based on `NoteLockAttribute`<br />
<br />
Also, remove `selectionModel` from `KJotsBrowser` as it's not used there anyways.</span></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R573 KJots</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29148">https://phabricator.kde.org/D29148</a></div></div><br /><div><strong>To: </strong>poboiko, dvratil<br /><strong>Cc: </strong>kde-pim, fbampaloukas, dcaliste, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil<br /></div>