bug#33887: 26.1; Emacs hangs for several seconds when going to the end of an XML file in nXML mode

[Noam Postavsky]

Vincent Lefevre <vincent@vinc17.net> writes:

> There's still an issue. On the following XML file
>
> <root>
> <a>text</a>
> <!-- ' -->
> <a>text</a>
> </root>
>
> the part after the comment <!-- ' --> is highlighted as a comment.

> And another one:
>
> <root>
> <a>text</a>
> <!-- "don't" -->
> <a>text</a>
> </root>
>
> The second text is highlighted as a comment.

Right, this is a collision between the syntax rules.  The following
patch fixes it, though perhaps it would be better to just search for the
end of the comment in the ("\\(<\\)!--" (1 "< b")) rule instead?

>From a866e4f4b556fb4a346fa68c62296f10966690a1 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 18 May 2019 13:18:19 -0400
Subject: [PATCH] Fix sgml syntax handling of quotes in comments

* lisp/textmodes/sgml-mode.el (sgml-syntax-propertize-rules): Make
sure not to skip over comment ender when searching for quotes.
* test/lisp/textmodes/sgml-mode-tests.el (sgml-tests--quotes-syntax):
Add a some more cases.
---
 lisp/textmodes/sgml-mode.el            | 11 ++++++++---
 test/lisp/textmodes/sgml-mode-tests.el | 16 +++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index e3cf56aa0e..1af1d1eaef 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -350,9 +350,14 @@ sgml-font-lock-keywords
            ;; Be careful to call `syntax-ppss' on a position before the one
            ;; we're going to change, so as not to need to flush the data we
            ;; just computed.
-           (if (prog1 (zerop (car (syntax-ppss (match-beginning 0))))
-                 (goto-char (1- (match-end 0))))
-               (string-to-syntax ".")))))
+           (let ((ppss (syntax-ppss (match-beginning 0))))
+             (if (prog1 (zerop (car ppss)) ; Outside tag.
+                   (goto-char (1- (match-end 0)))
+                   ;; If we're in a comment, don't skip over comment
+                   ;; ender.
+                   (when (nth 4 ppss)
+                     (skip-chars-backward "- \t\n")))
+                (string-to-syntax "."))))))
      )))
 
 (defun sgml-syntax-propertize (start end)
diff --git a/test/lisp/textmodes/sgml-mode-tests.el 
b/test/lisp/textmodes/sgml-mode-tests.el
index ffcc2cd840..7e1ddf4047 100644
--- a/test/lisp/textmodes/sgml-mode-tests.el
+++ b/test/lisp/textmodes/sgml-mode-tests.el
@@ -166,13 +166,15 @@ sgml-with-content
                  "<t>\"a'</t>"
                  "<t>'a\"</t>"
                  "<t>\"a'\"</t>"
-                 "<t>'a\"'</t>"))
-   (with-temp-buffer
-     (sgml-mode)
-     (insert str)
-     ;; Check that last tag is parsed as a tag.
-     (should (= 1 (car (syntax-ppss (1- (point-max))))))
-     (should (= 0 (car (syntax-ppss (point-max))))))))
+                 "<t>'a\"'</t>"
+                 "<t><!-- ' --></t>"
+                 "<t><!-- \" --></t>"))
+    (ert-info (str :prefix "Test string: ")
+      (sgml-with-content
+       str
+       ;; Check that last tag is parsed as a tag.
+       (should (= 1 (car (syntax-ppss (1- (point-max))))))
+       (should (= 0 (car (syntax-ppss (point-max)))))))))
 
 (provide 'sgml-mode-tests)
 ;;; sgml-mode-tests.el ends here
-- 
2.11.0

> <!DOCTYPE root [
> <!ENTITY f SYSTEM "f.xml">
> ]>
> <root>
> <a>ab'cd</a>
> <a>text</a>
> </root>

This is a different issue, I think the problem is that
sgml-syntax-propertize-inside doesn't handle nesting in the DTD
definition <! [ <! ... > ]>.  The patch below just avoids calling
sgml-syntax-propertize-inside on the prolog in nxml-mode (but the
problem remains in sgml-mode).  Though you'll hit Bug#18871/23668 if you
try to edit the DTD.

>From 9a50fc38b537d570f739c428a57c66557152151b Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 18 May 2019 14:37:51 -0400
Subject: [PATCH] Don't sgml-syntax-propertize-inside XML prolog

* lisp/nxml/nxml-mode.el (nxml-syntax-propertize): New function.
(nxml-mode): Use it as the syntax-propertize-function.
* test/lisp/nxml/nxml-mode-tests.el (nxml-mode-doctype-and-quote-syntax):
New test.
---
 lisp/nxml/nxml-mode.el            | 16 +++++++++++++++-
 test/lisp/nxml/nxml-mode-tests.el |  8 ++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index ab035b927e..7c39c5023c 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -423,6 +423,20 @@ nxml-parent-document-set
     (when rng-validate-mode
       (rng-validate-while-idle (current-buffer)))))
 
+(defvar nxml-prolog-end) ;; nxml-rap.el
+(defun nxml-syntax-propertize (start end)
+  "Syntactic keywords for `nxml-mode'."
+  ;; Like `sgml-syntax-propertize', but skip prolog.
+  (setq start (max start nxml-prolog-end))
+  (if (>= start end)
+      (goto-char end)
+    (goto-char start)
+    (sgml-syntax-propertize-inside end)
+    (funcall
+     (syntax-propertize-rules sgml-syntax-propertize-rules)
+     start end)))
+
+
 (defvar tildify-space-string)
 (defvar tildify-foreach-region-function)
 
@@ -518,7 +532,7 @@ nxml-mode
        (nxml-with-invisible-motion
          (nxml-scan-prolog)))))
   (setq-local syntax-ppss-table sgml-tag-syntax-table)
-  (setq-local syntax-propertize-function #'sgml-syntax-propertize)
+  (setq-local syntax-propertize-function #'nxml-syntax-propertize)
   (add-hook 'change-major-mode-hook #'nxml-cleanup nil t)
 
   ;; Emacs 23 handles the encoding attribute on the xml declaration
diff --git a/test/lisp/nxml/nxml-mode-tests.el 
b/test/lisp/nxml/nxml-mode-tests.el
index 92744be619..2bbf92bc96 100644
--- a/test/lisp/nxml/nxml-mode-tests.el
+++ b/test/lisp/nxml/nxml-mode-tests.el
@@ -78,5 +78,13 @@ nxml-mode-tests-correctly-indented-string
       (should-not (equal (get-text-property squote-txt-pos 'face)
                          (get-text-property dquote-att-pos 'face))))))
 
+(ert-deftest nxml-mode-doctype-and-quote-syntax ()
+  (with-temp-buffer
+    (insert "<!DOCTYPE t [\n<!ENTITY f SYSTEM \"f.xml\">\n]>\n<t>'</t>")
+    (nxml-mode)
+    ;; Check that last tag is parsed as a tag.
+    (should (= 1 (car (syntax-ppss (1- (point-max))))))
+    (should (= 0 (car (syntax-ppss (point-max)))))))
+
 (provide 'nxml-mode-tests)
 ;;; nxml-mode-tests.el ends here
-- 
2.11.0