Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mobile: Fixes #11317: Fix race condition which may cause data loss, particularly before or after pasting text in the note editor #11334

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

mrjo118
Copy link
Contributor

@mrjo118 mrjo118 commented Nov 7, 2024

There have been various issues raised regarding the latest input text intermittently being lost on a note on the mobile app. I have managed to find a way to consistently recreate this issue.

By adding additional logging at various places and then reproducing the issue, I was able to figure out the root cause of the issue. The issue is caused by a race condition in saveNoteButton_press.
If the execution of the method overlaps, when "BaseModel.diffObjectsFields(comp.state.lastSavedNote, note)" is executed, it will incorrectly think that the note has not changed, when the value of comp.state.note has changed between entering the method and setting the stateNote field. Therefore the subsequent logic to save the note to the database will not save changes updated after saving the note, but before setting stateNote.

The fix for this is to simply set the lastSavedNote value (used for comparing the previous note state) to use the 'savedNote' value, rather than the 'note' value which may have newer changes than 'savedNote'. See #11317 for a video of the issue being reproduced.

EDIT: an additional change was required to ensure that if the note was exited quickly after making the last change, no input data would be lost. The updated video demonstrates no input being lost after a rapid exit of the note after making changes.

Attached are videos of the behaviour following the code change.

Exit note slowly, showing latest input renders correctly in view mode when exiting edit mode:
https://github.com/user-attachments/assets/e2af19b2-3296-40f1-9ad9-7b8f2b9c4cff
Exit note quickly, without waiting for note to re-render:
https://github.com/user-attachments/assets/b0df0a13-7811-463e-9ad1-97ceab8b8202

PLEASE NOTE: The CodeMirror error which pops up at the start of the video is pre-existing, and happens sometimes when opening very large notes in edit mode. But the error popup is not visible when running the published app (but the error can still be found in the logs) rather than a dev build

@personalizedrefrigerator personalizedrefrigerator added mobile All mobile platforms editor labels Nov 8, 2024
@personalizedrefrigerator
Copy link
Collaborator

personalizedrefrigerator commented Nov 8, 2024

I've deployed a version of this pull request to https://personalizedrefrigerator.github.io/joplin/web-client/.

PR feedback

Co-authored-by: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com>
@mrjo118 mrjo118 changed the title Mobile: Fixes #11317: Fix race condition which may cause data loss after pasting a link in the note editor Mobile: Fixes #11317: Fix race condition which may cause data loss, particularly before or after pasting text in the note editor Nov 12, 2024
@laurent22
Copy link
Owner

That's looks good, thanks @mrjo118 and @personalizedrefrigerator for looking into this!

@laurent22 laurent22 merged commit 11b3347 into laurent22:dev Nov 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor mobile All mobile platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants