| rkflx |
Unfortunately, poppler (the only backed that supports saving) is not able
to save properly if the file is modified by a third party while it is opened
So we give the user a warning saying things went wrong and give him the option
to not reload/close, that way if there was something very important in the annotations
she added she can try to save them (even if by copy&paste the contents to a third program)
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| part.cpp | ||
|---|---|---|
| 1720 | How about this? "There are unsaved changes, but the file has been modified by another program since it was opened.<br>Which version would you like to keep?" [choices would then be "Keep Okular version" and "Keep externally-modified version"] | |
| 1728 | How about this? "There are unsaved changes, but the file has been modified by another program since it was opened.<br>The file can no longer be saved and your changes will be lost.<br>Do you want to continue closing the file?" | |
| 2489 | How about this? "The file has been changed by another program since it was opened, which means it can no longer be saved." | |
| part.cpp | ||
|---|---|---|
| 1720 | I dont' like this wording, seems like it will let you not lose your changes if you say "keep okular version". All you can do is "hopefully" still see what is rendered, but doing much will probably also break really. | |
| part.cpp | ||
|---|---|---|
| 1720 | With "Keep Okular version", could we make it save over the file again using the copy in memory--including the current state of the user's annotations and forms? If not, then how about this? "There are unsaved changes, but the file has been modified by another program since it was opened and cannot be saved. Reloading will display the modified version of the file without any of your changes." | |
| part.cpp | ||
|---|---|---|
| 1720 | There's no such thing as "the copy in memory" (at least for PDF that is really what matters at this stage) "modified" is not clear if it's the old or new version, something like "new version" makes more sense | |
Thanks for working on my "complaint" ;)
In D8642#166462 I claimed that Save As worked correctly. After testing more, this turns out to be an edge case, i.e. it works (as in: annotation and document are saved) when overwriting with the original file. Obviously that's not applicable in general.
What is really irritating and frustrating here is that the user can still see the file (at least the cached pages, with the rest turning blank) and move around, "it's just there" but cannot be saved. This is in stark contrast to Kate and LibreOffice, where you are allowed to save back to disk:
I wonder how they are doing this? Are they reading the whole file to memory (apparently we do not for good reasons)? Could we just do a cp --reflink=auto? Maybe this is something to come back to in case we get a lot of angry users, but I think the warning dialogs are fine for now…
Code looks and works good.
As for the wording of the dialogs, in general it is recommended to:
With this, Albert's comments, Nate's suggestions and the screenshots from above in mind, here is how I would word the dialogs (@ngraham please check :)
| part.cpp | ||
|---|---|---|
| 1721 | Use title case. | |
| 1722 | For consistency with the Closing buttons: | |
| 1730 | Use title case (3×). | |
| 2488 | Use title case. | |
This is in stark contrast to Kate and LibreOffice, where you are allowed to save back to disk:
Right, that's what I originally thought this was all about. If the user can't, then this whole interface is almost rubbing salt in the wound: they're informed that they're in a situation they can't escape from and given no good options. If possible, we should really try to find a way to offer an option to write the working copy in Okular back to disk, overwriting what the other program did.
- "There are unsaved changes, and the file '<filename>' has been modified by another program. Your changes will be lost, because the file can no longer be saved.<br>Do you want to continue {closing | reloading} the file?"
- The file '<filename>' has been modified by another program, which means it can no longer be saved.
Fine with me, if this really is the best we can do for now.
| part.cpp | ||
|---|---|---|
| 1720 | That sounds fine. | |
Thanks for proofreading.
There is no such thing. You'll see this when opening a multi-page document and overwriting it with something completely different. After this, when moving to the next page or when zooming you will only see blank pages (plus what you added yourself). Okular/Poppler only has cached tiles, but not the actual data.
Do we have a bugzilla to save annotations to an external file? FDF or something (not sure whether only for forms or also annotations)? In that case, we could at least refer to that if this was implemented.
New text based in suggestions
Please aprove ASAP since we're past the message freeze already, the translators have approved for the exception but every day that passes makes it worse for them
They are given the option to stop the reload to try to salvage what they can, current status is they are given the option to save and then it breaks, obviously this is better, right?
If possible, we should really try to find a way to offer an option to write the working copy in Okular back to disk, overwriting what the other program did.
This has been happening forever and i don't see a single bug about this, let's please not make a huge deal of what seems to be a real corner case people don't seem to be hitting.
Thanks, looks good. You missed a single spot, but I guess you just wanted to test whether I check everything indeed ;)
| part.cpp | ||
|---|---|---|
| 1721 | Ping (neither done, nor marked as done). | |