From 8ad05196ad69eab6ba5ba559734e0545b1d423ef Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 28 Nov 2024 14:49:32 +0100 Subject: [PATCH 1/2] refactor: Remove obsolete `hasUnsavedChanges` logic It used to check if the last saved version of the document is older than the current version. But `document.currentVersion` no longer exists in the backend response, so this didn't work since quite some time. Signed-off-by: Jonas --- src/components/Editor.vue | 2 +- src/components/Editor/Status.vue | 7 ++----- src/services/SyncService.js | 1 + 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 75f2c1299ba..bf84542b129 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -642,7 +642,7 @@ export default { this.emit('ready') } if (Object.prototype.hasOwnProperty.call(state, 'dirty')) { - // ignore initial loading and other automated changes + // ignore initial loading and other automated changes before first user change if (this.$editor && (this.$editor.can().undo() || this.$editor.can().redo()) ) { diff --git a/src/components/Editor/Status.vue b/src/components/Editor/Status.vue index e211192edcb..0c012fcebcc 100644 --- a/src/components/Editor/Status.vue +++ b/src/components/Editor/Status.vue @@ -86,22 +86,19 @@ export default { return this.dirtyStateIndicator ? t('text', 'Saving …') : t('text', 'Saved') }, dirtyStateIndicator() { - return this.dirty || this.hasUnsavedChanges + return this.dirty }, lastSavedStatusTooltip() { let message = t('text', 'Last saved {lastSave}', { lastSave: this.lastSavedString }) if (this.hasSyncCollission) { message = t('text', 'The document has been changed outside of the editor. The changes cannot be applied.') } - if (this.dirty || this.hasUnsavedChanges) { + if (this.dirty) { message += ' - ' + t('text', 'Unsaved changes') } return message }, - hasUnsavedChanges() { - return this.document && this.document.lastSavedVersion < this.document.currentVersion - }, hasSyncCollission() { return this.syncError && this.syncError.type === ERROR_TYPE.SAVE_COLLISSION }, diff --git a/src/services/SyncService.js b/src/services/SyncService.js index 61860b18fd9..3fada85279d 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -218,6 +218,7 @@ class SyncService { this.emit('error', { type: ERROR_TYPE.PUSH_FORBIDDEN, data: {} }) } // TODO: does response.data ever have a document? maybe for errors? + // TODO: `currentVersion` is always 0 nowadays. Check if this is still needed. // Only emit conflict event if we have synced until the latest version if (response.data.document?.currentVersion === this.version) { this.emit('error', { type: ERROR_TYPE.PUSH_FAILURE, data: {} }) From c6990dcb45e68527a68ee71b7a203c2c83150849 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 28 Nov 2024 14:52:00 +0100 Subject: [PATCH 2/2] feat(SessionApi): Send save request via `sendBeacon` at `beforeunload` This will send a final save request on unsaved changes via the browsers native `navigator.sendBeacon()` function when navigating away from the website or the tab/browser is closed. Fixes: #6606 Implementation details: * While `beforeunload` event is less reliable than `visibilitychange` according to different sources on the internet, tests in Firefox on Linux desktop revealed that `visibilitychange` event doesn't fire when opening a new website in the same tab. `beforeunload` on the other hand reliably worked when switching web page and closing tab/browser. * We add and remove the `beforeunload` event with every `dirty` state change because according to MDN documentation websites with a registered `beforeunload` event don't benefit from bfcache optimization. See https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event Signed-off-by: Jonas --- src/components/Editor.vue | 11 +++++++++++ src/services/SessionApi.js | 30 ++++++++++++++++++++++-------- src/services/SyncService.js | 11 +++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index bf84542b129..cb2ffaaa329 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -327,6 +327,13 @@ export default { this.contentWrapper = this.$refs.contentWrapper }) }, + dirty(val) { + if (val) { + window.addEventListener('beforeunload', this.saveBeforeUnload) + } else { + window.removeEventListener('beforeunload', this.saveBeforeUnload) + } + }, }, mounted() { if (this.active && (this.hasDocumentParameters)) { @@ -835,6 +842,10 @@ export default { }) this.translateModal = false }, + + saveBeforeUnload() { + this.$syncService?.saveViaSendBeacon() + }, }, } diff --git a/src/services/SessionApi.js b/src/services/SessionApi.js index 45ff771b69b..2e020e37106 100644 --- a/src/services/SessionApi.js +++ b/src/services/SessionApi.js @@ -3,6 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ import axios from '@nextcloud/axios' +import { getRequestToken } from '@nextcloud/auth' import { generateUrl } from '@nextcloud/router' export class ConnectionClosedError extends Error { @@ -106,17 +107,30 @@ export class Connection { }) } - save({ version, autosaveContent, documentState, force, manualSave }) { - return this.#post(this.#url(`session/${this.#document.id}/save`), { + save(data) { + const url = this.#url(`session/${this.#document.id}/save`) + const postData = { ...this.#defaultParams, filePath: this.#options.filePath, baseVersionEtag: this.#document.baseVersionEtag, - version, - autosaveContent, - documentState, - force, - manualSave, - }) + ...data, + } + + return this.#post(url, postData) + } + + saveViaSendBeacon(data) { + const url = this.#url(`session/${this.#document.id}/save`) + const postData = { + ...this.#defaultParams, + filePath: this.#options.filePath, + baseVersionEtag: this.#document.baseVersionEtag, + ...data, + requestToken: getRequestToken() ?? '', + } + + const blob = new Blob([JSON.stringify(postData)], { type: 'application/json' }) + return navigator.sendBeacon(url, blob) } push({ steps, version, awareness }) { diff --git a/src/services/SyncService.js b/src/services/SyncService.js index 3fada85279d..8b59eb53013 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -300,6 +300,17 @@ class SyncService { } } + saveViaSendBeacon() { + this.#connection.saveViaSendBeacon({ + version: this.version, + autosaveContent: this._getContent(), + documentState: this.getDocumentState(), + force: false, + manualSave: true, + }) + logger.debug('[SyncService] saved using sendBeacon') + } + forceSave() { return this.save({ force: true }) }