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

From: Noam Postavsky <[email-masked]>
Date: Sat, 18 May 2019 00:15:39 -0400

Vincent Lefevre <[email-masked]> writes:

> I've tried the combination of
>
>   ca14dd1d4628094dd33d5d94694dcf5f29e843b8
>   7dab3ee7ab54b3c2e7bc24170376054786c01d6f
>
> and this patch against Debian's current source package.
>
> Emacs no longer hangs, but I get incorrect highlighting,
> for instance on the following XML file.
>
> <root>
> <!-- comment -->
> <a>"a'</a>
> <!-- comment -->
> </root>
>
> Highlighting starts to be wrong at the single-quote character.
> I've attached a screenshot obtained with the -Q option.
>
> Did I miss anything?

Ah, I didn't get the mixed quote handling right.  Here's the fix for master:

>From 4677edd8dd65b5d956732821e78794f35b275418 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[email-masked]>
Date: Sat, 18 May 2019 00:04:01 -0400
Subject: [PATCH] Fix Bug#33887 for mixed quote usage

* lisp/textmodes/sgml-mode.el (sgml-syntax-propertize-rules): Only
skip syntax-ppss for matched quotes.
* test/lisp/textmodes/sgml-mode-tests.el (sgml-tests--quotes-syntax):
Expand test.
---
 lisp/textmodes/sgml-mode.el            |  4 ++--
 test/lisp/textmodes/sgml-mode-tests.el | 17 ++++++++++++-----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 1b064fb825..e3cf56aa0e 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -345,8 +345,8 @@ sgml-font-lock-keywords
      ;; the resulting number of calls to syntax-ppss made it too slow
      ;; (bug#33887), so we're now careful to leave alone any pair
      ;; of quotes that doesn't hold a < or > char, which is the vast majority.
-     ("\\(?:\\(?1:\"\\)[^\"<>]*[<>\"]\\|\\(?1:'\\)[^'<>]*[<>']\\)"
-      (1 (unless (memq (char-before) '(?\' ?\"))
+     ("\\([\"']\\)[^<>\"']*[<>\"']"
+      (1 (unless (eq (char-after (match-beginning 1)) (char-before))
            ;; 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.
diff --git a/test/lisp/textmodes/sgml-mode-tests.el b/test/lisp/textmodes/sgml-mode-tests.el
index a900e8dcf2..ffcc2cd840 100644
--- a/test/lisp/textmodes/sgml-mode-tests.el
+++ b/test/lisp/textmodes/sgml-mode-tests.el
@@ -161,11 +161,18 @@ sgml-with-content
       (should (string= "&&" (buffer-string))))))
 
 (ert-deftest sgml-tests--quotes-syntax ()
-  (with-temp-buffer
-    (sgml-mode)
-    (insert "a\"b <tag>c'd</tag>")
-    (should (= 1 (car (syntax-ppss (1- (point-max))))))
-    (should (= 0 (car (syntax-ppss (point-max)))))))
+  (dolist (str '("a\"b <t>c'd</t>"
+                 "a'b <t>c\"d</t>"
+                 "<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))))))))
 
 (provide 'sgml-mode-tests)
 ;;; sgml-mode-tests.el ends here
-- 
2.11.0

And the correponding patch against emacs-26:

>From 3a1a36b0b42772f35c70fb7e996ba8fed787e1c2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[email-masked]>
Date: Wed, 15 May 2019 18:51:30 -0400
Subject: [PATCH] Backport sgml-syntax-propertize-rules speedup (Bug#33887)

* lisp/textmodes/sgml-mode.el (sgml-syntax-propertize-rules): Reapply
2019-01-17 "* lisp/textmodes/sgml-mode.el: Try and fix bug#33887."
taking into account 2019-05-09 "Recognize single quote attribute
values in nxml and sgml (Bug#35381)" which means we have to handle
single quotes as well.
* test/lisp/textmodes/sgml-mode-tests.el (sgml-quote-works): New test.
---
 lisp/textmodes/sgml-mode.el            | 21 +++++++++++++++------
 test/lisp/textmodes/sgml-mode-tests.el | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 128e58810e..f8a37c3820 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -347,12 +347,21 @@ sgml-font-lock-keywords
      ("--[ \t\n]*\\(>\\)" (1 "> b"))
      ("\\(<\\)[?!]" (1 (prog1 "|>"
                          (sgml-syntax-propertize-inside end))))
-     ;; Quotes outside of tags should not introduce strings.
-     ;; 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.
-     ("[\"']" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0))))
-                       (goto-char (match-end 0)))
-                     (string-to-syntax ".")))))))
+     ;; Quotes outside of tags should not introduce strings which end up
+     ;; hiding tags.  We used to test every quote and mark it as "."
+     ;; if it's outside of tags, but there are too many quotes and
+     ;; the resulting number of calls to syntax-ppss made it too slow
+     ;; (bug#33887), so we're now careful to leave alone any pair
+     ;; of quotes that doesn't hold a < or > char, which is the vast majority.
+     ("\\([\"']\\)[^<>\"']*[<>\"']"
+      (1 (unless (eq (char-after (match-beginning 1)) (char-before))
+           ;; 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 ".")))))
+     )))
 
 (defun sgml-syntax-propertize (start end)
   "Syntactic keywords for `sgml-mode'."
diff --git a/test/lisp/textmodes/sgml-mode-tests.el b/test/lisp/textmodes/sgml-mode-tests.el
index 7318a667b3..8d0bb88163 100644
--- a/test/lisp/textmodes/sgml-mode-tests.el
+++ b/test/lisp/textmodes/sgml-mode-tests.el
@@ -130,5 +130,19 @@ sgml-with-content
    (sgml-delete-tag 1)
    (should (string= "Winter is comin'" (buffer-string)))))
 
+(ert-deftest sgml-tests--quotes-syntax ()
+  (dolist (str '("a\"b <t>c'd</t>"
+                 "a'b <t>c\"d</t>"
+                 "<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))))))))
+
 (provide 'sgml-mode-tests)
 ;;; sgml-mode-tests.el ends here
-- 
2.11.0