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

File open always triggers save and overwrites last modified date #2337

Closed
relikd opened this issue Apr 25, 2022 · 16 comments · Fixed by #2686
Closed

File open always triggers save and overwrites last modified date #2337

relikd opened this issue Apr 25, 2022 · 16 comments · Fixed by #2686
Labels
1. to develop bug Something isn't working

Comments

@relikd
Copy link

relikd commented Apr 25, 2022

Describe the bug
I have multiple files which are saved as soon as they are opened. I am not sure why these files are updated. I tried to reproduce the bug with a new file but the new file was not affected. Maybe something with version history.

However, the network log shows a needless push "stepType":"replace","from":35,"to":35

Expected behavior
The file should not be written to disk if it has no changes. The unnecessary write destroys the "last changed" date.

Screenshots

Untitled.mp4

Client details:

  • OS: macOS
  • Browser: Firefox
Server details

Text app version: 3.4.0

Operating system: Arch

Web server: nginx

Database: mysql 8

PHP version: 8

Nextcloud version: 23.0.2 and 23.0.3

Logs

Nextcloud log (data/nextcloud.log)

N/A

Browser log

$ is deprecated: The global jQuery is deprecated. It will be removed in a later versions without another warning. Please ship your own. 9 main.js:1358:3634
Opening viewer for file  /Briefkasten/Protokoll-2022-01-13.md viewer-main.js:2:993487
oc_defaults is deprecated: use OC.theme instead, this will be removed in Nextcloud 20 main.js:1358:3634
Fetching additional files... viewer-main.js:2:994064
receivedSteps newVersion 1 editor.js:2:132079
receivedSteps newVersion 2 editor.js:2:132079
Saved document 
Object { id: <redacted>, currentVersion: 2, lastSavedVersion: 2, lastSavedVersionTime: 1650926125, baseVersionEtag: "<redacted>", initialVersion: 0 }
editor.js:2:124054
No new notification data received notifications-main.js:2:1120986
Polling interval updated to 30000 notifications-main.js:2:1121750
OC.Util.relativeModifiedDate is deprecated and will be removed in Nextcloud 21. See @nextcloud/moment main.js:116:2282
POST https://<server>/index.php/apps/text/session/push

{"documentId":<redacted>,"sessionId":<redacted>,"sessionToken":"<redacted>","steps":[{"stepType":"replace","from":35,"to":35,"slice":{"content":[{"type":"paragraph"}]}}],"version":0,"token":null,"filePath":"/Briefkasten/Protokoll-2022-01-13.md"}
@relikd relikd added the bug Something isn't working label Apr 25, 2022
@relikd
Copy link
Author

relikd commented Apr 25, 2022

Okay, finding the problem was unexpectedly easy, steps to reproduce:

  1. create new file with content:
### 

(thats 5 characters, "###" + space + newline)

  1. open the file

UPDATE: it has nothing to do with the h3 heading. The root cause is that Nextcloud Text expects the file to end in a paragraph. But, whenever the last text is in a heading (this example), or a bullet list, or enumeration, then the file will be saved again and again. A workaround is to insert a plain paragraph at the end.

@githubkoma
Copy link

githubkoma commented May 4, 2022

Thank god (sic) i am not the only one with this issue, it melt my mind for 1-2 days of my weekend and now you brought the issue on point!

The following is to confirm you findings somehow, but for me i cant reliably work around it, this bugs me in my usecase of using many Text Files :-(

i'm having the same issue but i am not aware how to fully reproduce it, it seems this time that when i did paste "some stuff" into the editor, it starts to break and wants to save all the time when opening the file again. So for now i have "somehow" created two files:

  • 1 saving all the time when opening the editor
  • The other 1 that is not saving when opening it

File with the problem:

[root@9e2443edc6b5 .PadItems]# cat -TEAv .PAD102.md
### Header3$
$
* [ ] Todo1$
* [ ] Todo2$
* [x] Todo3$
* [ ] Todo4$
* [ ] Todo5$
* [ ] Todo6$
* [ ] Todo7$
* [ ] [root@9e2443edc6b5 .PadItems]#

File NOT having the issue:

[root@9e2443edc6b5 .PadItems]# cat -TEAv .PAD103.md
### Header3$
$
* [ ] Todo1[root@9e2443edc6b5 .PadItems]#

UPDATE:
Second File is now also having the problem, but i didnt write/paste anything into it in the meantime...

[root@9e2443edc6b5 .PadItems]# cat -TEAv .PAD103.md
### Header3$
$
* [ ] Todo1[root@9e2443edc6b5 .PadItems]#

And i can confirm that the stuff the editor changes on opening is inserting a new paragraph:

  • Request URL: https://.../apps/text/session/push
  • Request Method: POST

Payload:

[{"stepType":"replace","from":30,"to":30,"slice":{"content":[{"type":"paragraph"}]}}]

@relikd
Copy link
Author

relikd commented May 4, 2022

@githubkoma A workaround is to add a newline and a dot at the end. Or any other character, it doesnt matter. The dot will create a new paragraph and prevent this issue.

### Header3$
$
* [ ] Todo1[root@9e2443edc6b5 .PadItems]$
$
.#

Hopefully this will be fixed soon.

@githubkoma
Copy link

githubkoma commented May 4, 2022

@relikd thanks! (i just had to wait a few seconds after adding the dot and new line until it "settled")

@DeaR devs: this is the above listed first file that has the issue:
PAD102.md

@juliushaertl
Copy link
Member

I think this might be related to the TrailingNode plugin that we use, which also mentions that the node is not only visual:

Issues

This implementation adds an actual node. It’d be great to use a decoration for that use case, so the document isn’t modified.

@max-nextcloud I'm wondering if we should rather strip any trailing node during the markdown conversion then to avoid rewriting the file?

@noseshimself
Copy link

noseshimself commented May 27, 2022

If it was only writing the exact contents back it would be an inconvenience. But just opening some files modifies them immediately, see #2344 .

@max-nextcloud
Copy link
Collaborator

@max-nextcloud I'm wondering if we should rather strip any trailing node during the markdown conversion then to avoid rewriting the file?

Yes... Either that or we could maybe overwrite the toMarkdown function of the trailing node to return an emtpy string. Not sure if that would work though as it would still be a new node. Might be worth a try never the less.

@juliushaertl
Copy link
Member

Pushed a hot fix to #2686

Turned out that the trailing node is actually n to part of the markdown and therefore being the first thing that gets added with the first dispatched transaction (focus).

@meonkeys
Copy link
Contributor

meonkeys commented Dec 8, 2022

Are we sure this is fixed? I upgraded from 24.0.7 to 25.0.1 last night. Since then I'm seeing files marked as modified when I do not make any changes. I assumed it was related to link unfurling / previews.

@max-nextcloud
Copy link
Collaborator

@meonkeys do you still have some files that you did not open?
Could you copy the file first, then open it and close without changing.
If the file content changed that might give a clue to why this is happening.

@meonkeys
Copy link
Contributor

meonkeys commented Dec 8, 2022

@max-nextcloud I do, and I tried what you said. But now I can't reproduce this. I just set 'reference_opengraph' => false in my config, so I'm still wondering if link unfurling / previews in v25 might have been causing files to appear as modified. 🤷

UPDATE sorry, correction, I think I can reproduce this. Files with zero bare links are correctly left unmodified if I open them and make zero changes. Files with bare links are marked as modified. I think. This is a tricky one and I don't understand how to debug it.

@max-nextcloud
Copy link
Collaborator

@meonkeys Could you give an example of a bare link? Something like https://nextcloud.com ?

@meonkeys
Copy link
Contributor

meonkeys commented Dec 8, 2022

Sure. And sorry, I may have made up the term "bare link". Here's what I mean:

bare link: https://nitter.net/meonkeys

non-bare link: [Adam Monsen on Twitter, via Nitter](https://nitter.net/meonkeys)

I picked this URL because the target HTML data includes OpenGraph metadata.

@relikd
Copy link
Author

relikd commented Dec 8, 2022

I do not know how backports work. Should this be fixed in v24? And if so, which minor version? (I can say, it does not work with 24.0.6)

@susnux
Copy link
Contributor

susnux commented Dec 8, 2022

I do not know how backports work. Should this be fixed in v24? And if so, which minor version? (I can say, it does not work with 24.0.6)

Either 25.0.0 or 24.0.8 (see https://nextcloud.com/changelog/ )

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Dec 9, 2022

Files with bare links are marked as modified.

I opened #3543 for this as the root cause seems to be different from the one that originally triggered #2337.
Let's continue the investigation there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants