Skip to content

fix(files): edited files marked locked in files list after close - #9355

Open
ShGKme wants to merge 1 commit into
mainfrom
fix/file-is-locked-after-close
Open

fix(files): edited files marked locked in files list after close#9355
ShGKme wants to merge 1 commit into
mainfrom
fix/file-is-locked-after-close

Conversation

@ShGKme

@ShGKme ShGKme commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

馃摑 Summary

Caused by:

text/src/components/CollaborativeEditor.vue

Lines 768 to 773 in a3daf37

onSave() {
if (this.fileNode) {
this.fileNode.mtime = new Date()
this.fileNode.size = new Blob([this.serialize()]).size
emit('files:node:updated', this.fileNode)
}

  1. After session is connected, the file is locked
  2. fetchFile fetches the locked file, stored later in this.fileNode
  3. On save, local file node copy is updated with the new size and mtime ...
  4. ... and sent to Files
  5. With the updated size/mtime, Files also receives attributes.lock = 1

From my understanding, the Text frontend doesn't actually know if the file is locked or not after the file is saved and the session connection is closed. /close endpoint may and may not unlock the file.

Instead of changing the lock and other properties locally, we can fetch the actual updated node state.
This way we have all the properties and attributes updated.

This, unfortunately, adds one more request.

I added it to the close method instead of onSave. onSave's cheap (no-request) update is kept for a case when the file is edited in the folder description (README.md). Only on viewer closed when the session is actually closed the file is updated in Files app completely.

This may result in flickering for a second on close:

  1. File is update to "locked" from onSave
  2. Then to "unlocked" on new node fetch in close().

IMO, we can:

  • Either remove update in onSave completely (but then there is no visual update on README from description)
  • Or add additional flag to check if this save is on close...

What fo you think?

馃弫 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

馃 AI (if applicable)

  • The content of this PR was partly or fully generated using AI tools
  • The AI-generated content was reviewed, comprehended and tested by a human

@ShGKme ShGKme self-assigned this Oct 8, 2026
@ShGKme ShGKme added bug Something isn't working 3. to review labels Oct 8, 2026
@benjaminfrueh

benjaminfrueh commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@ShGKme thank you for this, I didn't test but it makes sense to me to fetch it again on close.

I think the new fetchNode() request can race with the POST /close sent just before in this.disconnect() -> this.syncService.close(), because the close() call in SyncService.close() is not awaited. If the PROPFIND is handled before the server has released the lock (e.g. slow db), the file could stay marked as locked.

Awaiting it there could fix it but connection.value would then only be cleared after the response, maybe have to move that up and unsure about other side effects this could have. Curious what @max-nextcloud thinks about this.

@ShGKme

ShGKme commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

because the close() call in SyncService.close() is not awaited

Ah, yes, I see.
Can we make it awaited?

Something like:

async close() {
	const activeConnection = this.connection.value 

	this.backend?.disconnect()

	// Clear connection immediately so hasActiveConnection turns false and we can reconnect.
	this.connection.value = undefined
	this.bus.emit('close')

	if (activeConnection) {
		await close(activeConnection)
			// Log and ignore possible network issues.
			.catch((e) => {
				logger.info('Failed to close connection.', { e })
			})
	}
}

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud max-nextcloud is a code owner
@mejo- mejo- Awaiting requested review from mejo- mejo- is a code owner
@silverkszlo silverkszlo Awaiting requested review from silverkszlo silverkszlo is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@ShGKme ShGKme

Labels

3. to review bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

File is marked as "Locked by editing online in Text" in Files after closing edited file

2 participants

@ShGKme @benjaminfrueh