integreat-cms
integreat-cms copied to clipboard
Robust changed content detection
Short description
Rework changed content detection from using a dirty flag to comparing with the previous content and add a little indicator. With this we can reliably tell whether we need to autosave at all or not.
Proposed changes
- Remove dirty flag and determine presence of changes by comparing with previously captured state
- Only attatch beforeunload listener when dirty to allow the browsers back/forward cache (per recommendation: https://developer.chrome.com/articles/page-lifecycle-api/#the-beforeunload-event)
- Prepend bullet to document title when dirty
Side effects
-
When closing the tab with unsaved changes, an autosave is triggered. However, the popup asking whether to really close the page is shown before the server can respond. This leads to situations where the content is saved despite a warning being shown. This cannot be changed, since it's impossible to predict whether it will be successful or not. See also #2132 and #2435.
-
There is a potential for the value representing the last saved state to be out of sync with what was actually sent as an auto save. This is because of the separation of the component tracking changes for the whole form and the autosave plugin to tinymce which performs the actual autosave: the former only receives the signal that an autosave is being attempted and then that it was indeed successful, but never the actual data that was sent. It might theoretically be possible for the content to change in the tiny instance between the autosave gathering the data to send to the server and the unsaved warning module receiving the signal and itself gathering the data as a candidate for the next saved state. It might be possible to mitigate this by moving the autosave itself out of the editor plugin (then at most sending a signal to trigger it), since it requires the whole form data, which is technically out of the scope of a tinymce plugin (also more than one editor could be part of the same form, which would create problems, though we do this nowhere). However, this is hopefully very unlikely and should not do any damage if it should occur: The state would likely only be different by a single character until the next autosave or reload, and a wrong save decision because of it would not pose a problem be caught and corrected reasonably soon.
-
The change indicator in the title relies on the title not being changed (by JS) over the life cycle of a page in the browser. This is because to reliably add the bullet, the original title is saved in a variable. Currently, we don't even give a different title to different CMS pages.
Resolved issues
Fixes #2477
Code Climate has analyzed commit f6ee9732 and detected 0 issues on this pull request.
The test coverage on the diff in this pull request is 100.0% (50% is the threshold).
This pull request will bring the total coverage in the repository to 78.9% (0.0% change).
View more on Code Climate.
On second thought, there is no reason for this to require merging after #2401
On second thought, there is no reason for this to require merging after https://github.com/digitalfabrik/integreat-cms/pull/2401
@PeterNerlich so, is it still actual?
@PeterNerlich so, is it still actual?
Yes, feel free to review the PR and/or give an opinion on the feature with the bullet in the document title
Thank you for bringing up that issue! I totally missed that this happens. It was annoying to figure out how to fix, but this is how I think it occurred: TinyMCE hides the original text field it's supposed to represent and builds its own interface, requiring it to save its state back to the form element before the form data gets captured and sent in order to be up to date. This is taken care of for the autosave functionality, but this didn't happen for the regular form submit.
So in order to ensure this also for the normal submit, I added a formdata listener that modifies the formdata gathered on submit (and also when calling new FormData(form)
) to contain the latest tinymce content.
I can still reproduce the problem where changes made are lost after clicking the "Update" button. However, this appears to be a flaky problem and I don't have the exact steps to reproduce it.