Subject: Re: bug#36347: 26.2.90; fontification mistakes contraction for a string in HTML

From: Noam Postavsky <[email-masked]>
Date: Sun, 23 Jun 2019 21:47:00 -0400

Mike Kupfer <[email-masked]> writes:

> To reproduce:
>
> 1. start emacs with -Q
> 2. Visit (C-x C-f) the attached file (foo.html).
>
> When I do this with 26.2.90, the text starting with the apostrophe in
> "doesn't", and ending at the end of file, is fontified as a string.
>
> When I do this with 26.2-rc1, the text is fontified as I would expect
> (HTML tags are in blue, everything else is in the default black or dark
> gray).

The problem is the parens around the quote, sgml-mode uses the wrong
syntax table during propertizing.  The only reason it doesn't show up in
26.2 is because sgml-mode fails to fontify single quoted strings at all.
The same problem exists in 26.2 and earlier with double quote, although
it's less likely to have an unmatched double quote in text.

<html>(")</html>

<html>(')</html>

So the question is what to do about this for the release branch.

1. Revert the fix which adds handling of single quotes.  This means
single quotes never cause highlighting, even when they should.  This is
a regression in nxml-mode relative to Emacs 25, but sgml-mode worked
like that for a while already (in Emacs 26, nxml-mode was changed to
rely on some of sgml-mode's code).

2. Fix this bug with the patch below, it's fairly small and
straightforward.  And also, can be disabled by setting
syntax-ppss-table to nil in sgml-mode-hook.

3. Do nothing (for 26.3, that is).

>From cb2f5380e3e78b7d956c604c03c21122c8ced36d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[email-masked]>
Date: Sun, 23 Jun 2019 21:27:43 -0400
Subject: [PATCH] Fix sgml-mode handling of quotes within parens (Bug#36347)

* lisp/textmodes/sgml-mode.el (sgml-syntax-propertize): Use
syntax-ppss-table if set.  This is only needed on the release branch,
on master the caller (syntax-propertize) already does this.
(sgml-mode): Set syntax-ppss-table to sgml-tag-syntax-table.  This
correctly classifies parens as punctuation, so they won't confuse the
parser.
* test/lisp/textmodes/sgml-mode-tests.el (sgml-tests--quotes-syntax):
New test copied from master, with two cases added for this bug.
---
 lisp/textmodes/sgml-mode.el            | 10 ++++++----
 test/lisp/textmodes/sgml-mode-tests.el | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 128e58810e..895ce844e9 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -357,10 +357,11 @@ (eval-and-compile
 (defun sgml-syntax-propertize (start end)
   "Syntactic keywords for `sgml-mode'."
   (goto-char start)
-  (sgml-syntax-propertize-inside end)
-  (funcall
-   (syntax-propertize-rules sgml-syntax-propertize-rules)
-   start end))
+  (with-syntax-table (or syntax-ppss-table (syntax-table)) ; Not needed on master.
+    (sgml-syntax-propertize-inside end)
+    (funcall
+     (syntax-propertize-rules sgml-syntax-propertize-rules)
+     start end)))
 
 (defun sgml-syntax-propertize-inside (end)
   (let ((ppss (syntax-ppss)))
@@ -568,6 +569,7 @@ (define-derived-mode sgml-mode text-mode '(sgml-xml-mode "XML" "SGML")
 			      sgml-font-lock-keywords-2)
 			     nil t))
   (setq-local syntax-propertize-function #'sgml-syntax-propertize)
+  (setq-local syntax-ppss-table sgml-tag-syntax-table)
   (setq-local facemenu-add-face-function 'sgml-mode-facemenu-add-face-function)
   (setq-local sgml-xml-mode (sgml-xml-guess))
   (unless sgml-xml-mode
diff --git a/test/lisp/textmodes/sgml-mode-tests.el b/test/lisp/textmodes/sgml-mode-tests.el
index 7318a667b3..0000b352ff 100644
--- a/test/lisp/textmodes/sgml-mode-tests.el
+++ b/test/lisp/textmodes/sgml-mode-tests.el
@@ -130,5 +130,27 @@ (ert-deftest sgml-delete-tag-bug-8203-should-not-delete-apostrophe ()
    (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>"
+                 "a\"b <tag>c'd</tag>"
+                 ;;"<tag>c>'d</tag>" Fixed in master.
+                 "<t><!-- \" --></t>"
+                 "<t><!-- ' --></t>"
+                 "<t>(')</t>"
+                 "<t>(\")</t>"
+                 ))
+   (with-temp-buffer
+     (sgml-mode)
+     (insert str)
+     (ert-info ((format "%S" str) :prefix "Test case: ")
+       ;; 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