Subject: Re: bug#76478: 30.0.92; doc-view--revert-buffer fails when saving OpenDocument spreadsheets

From: Tassilo Horn <[email-masked]>
Date: Sun, 23 Feb 2025 10:08:38 +0100

Tassilo Horn <[email-masked]> writes:

>>> When I open an OpenDocument spreadsheet (file extension ods), by
>>> default this happens via doc-view.  Then, when I edit the file (say,
>>> content.xml) and try to save it, doc-view--revert-buffer bails out
>>> with the error (wrong-type-argument stringp nil).
>>> 
>>> If, instead, I open and edit the file as a zip archive without
>>> doc-view, I can save it without any problems.
>>
>> Thanks.
>>
>> Tassilo, could you please look into this?

You actually don't need an ODS document for that error to trigger.  A
PDF works, too.

  - Find foo.pdf
  - C-c C-c to switch to the editing mode
  - M-x revert-buffer RET ==> bang!

The difference with OpenDocuments is that after editing some file inside
the archive, the archive buffer will be reverted automatically.

The problem is that when switching to the editing mode, the buffer-local
value of doc-view--buffer-file-name is killed and so our
doc-view--revert-buffer advice function errors when trying to update the
cached copy of the document (which only exists if the document is remote
or inside some archive).

I've added a nil-check there.  I think that's ok.  Alternatively, we
could possibly remove the advice function when switching to the editing
mode.  Stefan, what do you think?

I've also found and fixed another related bug.  When switching to the
editing mode for an OpenDocument (archive-mode), then editing its
contents and saving (causing a revert), doc-view-minor-mode was
disabled, i.e., you couldn't C-c C-c back to doc-view-mode.  Here's
possibly also an alternative: the archive--mode-revert advice function
could bind revert-buffer-preserve-modes just as our
doc-view--revert-buffer does.  But I think my version is better because
otherwise we'd need to make all possible editing modes play to
doc-view's rules.

Stefan, do you think my patch is ok?

And Eli, in which version do you want the fix?  I think the issue is
quite old and as explained above, you will usually only trigger it when
editing OpenDocument contents which probably is quite uncommon.

Bye,
Tassilo

>From e06e58646dbd3df26d5a99b13d13f6d908529ee8 Mon Sep 17 00:00:00 2001
From: Tassilo Horn <[email-masked]>
Date: Sun, 23 Feb 2025 09:46:54 +0100
Subject: [PATCH] doc-view: Fix error during revert in editing mode

When in editing mode in a doc-view buffer and then reverting (which can
happen automatically when editing OpenDocument contents), we errored in
the advide function doc-view--revert-buffer because the local
doc-view--buffer-file-name has been killed when switching to the editing
mode.

Also restore doc-view-minor-mode after reverting during being in the
editing mode.

* lisp/doc-view.el (doc-view--revert-buffer): Check that
'doc-view--buffer-file-name' is non-nil.
(doc-view-minor-mode): Add re-enabling function to
'revert-buffer-restore-functions'.
---
 lisp/doc-view.el | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 98522f35765..501bf406ca4 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -591,11 +591,15 @@ doc-view--revert-buffer
   (cl-labels ((revert ()
                 (let ((revert-buffer-preserve-modes t))
                   (apply orig-fun args)
-                  ;; Update the cached version of the pdf file,
-                  ;; too.  This is the one that's used when
-                  ;; rendering (bug#26996).
-                  (unless (equal buffer-file-name
-                                 doc-view--buffer-file-name)
+                  ;; Update the cached version of the pdf file, too.
+                  ;; This is the one that's used when rendering
+                  ;; (bug#26996).  doc-view--buffer-file-name is nil in
+                  ;; the case where we've switched to the editing mode
+                  ;; (bug#76478).  In that case, we'll update the cached
+                  ;; version when switching back to doc-view-mode.
+                  (when (and doc-view--buffer-file-name
+                             (not (equal buffer-file-name
+                                         doc-view--buffer-file-name)))
                     ;; FIXME: Lars says he needed to recreate
                     ;; the dir, we should figure out why.
                     (doc-view-make-safe-dir doc-view-cache-directory)
@@ -2443,7 +2447,20 @@ doc-view-minor-mode
 See the command `doc-view-mode' for more information on this mode."
   :lighter " DocView"
   (when doc-view-minor-mode
-    (add-hook 'change-major-mode-hook (lambda () (doc-view-minor-mode -1)) nil t)
+    (add-hook 'change-major-mode-hook
+              (lambda ()
+                (doc-view-minor-mode -1))
+              nil t)
+    ;; OpenDocuments are archive files, so their editing mode is
+    ;; archive-mode.  When editing and saving a file in that archive,
+    ;; it'll automatically revert the archive buffer.  Take care to
+    ;; re-enable `doc-view-minor-mode' in that case.
+    (add-hook 'revert-buffer-restore-functions
+              (lambda ()
+                (lambda ()
+                  (unless (derived-mode-p 'doc-view-mode)
+                    (doc-view-minor-mode 1))))
+              nil t)
     (message
      "%s"
      (substitute-command-keys
-- 
2.48.1