bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz, Bunkus <moritz]

Hey,

I'm using Emacs to write DocBook XML documents that will be turned into
man pages later. In said XML documents I often define entities used
later in the file. As soon as the that list gets too long (~5
definitions), nxml-mode fails to parse the following structure properly
and fails to close open start tags with the error message "No matching
start-tag".

Note that nxml-mode always states that the document is valid, even
though it fails to find an open start tag (I'm storing DocBook relaxng
schemas in ~/.xml/schemas/relaxng/docbook/4.5/…, but the same is true
when the schemas from /usr/share/emacs/26.1/etc/schema/… are used).

How to reproduce:

1. Save the XML code below into a file.
2. Start emacs -Q
3. Open the test file.
4. Place the cursor at the end of the line starting with "<refentry…".
5. Try using the "nxml-finish-element" function, e.g. C-c C-f. It will
   show the aforementioned error message instead of inserting "</refentry>".
6. Now shorten the entity definition list, e.g. by removing the whole
   line defining "ent5". Execute "normal-mode" to let nxml-mode parse the
   buffer again. Re-run the test from step 5. It will now insert
   "</refentry>" properly.

---[ test file content start ]-----------------------------------------------------
<?xml version="1.0" encoding="utf-8"?>

<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"
[
<!ENTITY &product; "myprog">
<!ENTITY ent2 "moretext">
<!ENTITY ent3 "evenmoretext">

<!ENTITY ent4 "<citerefentry><refentrytitle>anotherprogram</refentrytitle><manvolnum>1</manvolnum></citerefentry>">
<!ENTITY ent5 "<citerefentry><refentrytitle>onemoreprogram</refentrytitle><manvolnum>1</manvolnum></citerefentry>">
]
>

<refentry lang="en">
 <refentryinfo>
  <productname>&product;</productname>
 </refentryinfo>

 <refmeta>
  <refentrytitle>&product;</refentrytitle>
 </refmeta>

 <refnamediv>
  <refname>&product;</refname>
  <refpurpose>doing things</refpurpose>
 </refnamediv>

 <refsect1>
  <title>Test</title>
  <para>here</para>
 </refsect1>
</refentry>
---[ test file content end ]-----------------------------------------------------

Kind regards
mosu

In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2018-07-05 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12001000
Recent messages:
Saving file /home/mbunkus/tmp/nxml-test.xml...
Wrote /home/mbunkus/tmp/nxml-test.xml
Using schema ~/.xml/schemas/relaxng/docbook/4.5/docbook.rnc
nxml-finish-element-1: No matching start-tag
Saving file /home/mbunkus/tmp/nxml-test.xml...
Wrote /home/mbunkus/tmp/nxml-test.xml
Using schema ~/.xml/schemas/relaxng/docbook/4.5/docbook.rnc
Undo! [2 times]
Saving file /home/mbunkus/tmp/nxml-test.xml...
Wrote /home/mbunkus/tmp/nxml-test.xml

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Eli Zaretskii]

> From: Moritz@chai-latte.bs.linet-services.de,
> 	"Bunkus <moritz"@bunkus.org
> Date: Mon, 24 Sep 2018 14:50:29 +0200
> 
> Note that nxml-mode always states that the document is valid

It says "invalid" here...

> 1. Save the XML code below into a file.
> 2. Start emacs -Q
> 3. Open the test file.
> 4. Place the cursor at the end of the line starting with "<refentry…".
> 5. Try using the "nxml-finish-element" function, e.g. C-c C-f. It will
>    show the aforementioned error message instead of inserting "</refentry>".
> 6. Now shorten the entity definition list, e.g. by removing the whole
>    line defining "ent5". Execute "normal-mode" to let nxml-mode parse the
>    buffer again. Re-run the test from step 5. It will now insert
>    "</refentry>" properly.

Maybe I'm not reproducing this correctly, but in my case it emits the
same error even after removing the ent5 line.

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz Bunkus]

Hey,

thanks for giving it a try, Eli.

> It says "invalid" here...

Doh, stupid me. I made a copy & paste error in my example file. The
offending line is:

<!ENTITY &product; "myprog">

which should read:

<!ENTITY product "myprog">

After that change the result will be valid again. Sorry about that.

> Maybe I'm not reproducing this correctly, but in my case it emits the
> same error even after removing the ent5 line.

If removing ent5 isn't enough, try removing more entries. You can also
remove all of the definitions including the [ … ] parenthesis, leaving you
with just

…
<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd">

<refentry lang="en">
…

At this point C-c C-f should work at the latest.

Kind regards,
mosu

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Eli Zaretskii]

> From: Moritz Bunkus <moritz@bunkus.org>
> Cc: 32823@debbugs.gnu.org
> Date: Mon, 24 Sep 2018 19:12:59 +0200
> 
> Doh, stupid me. I made a copy & paste error in my example file. The
> offending line is:
> 
> <!ENTITY &product; "myprog">
> 
> which should read:
> 
> <!ENTITY product "myprog">
> 
> After that change the result will be valid again. Sorry about that.

Right, this settled it.

> > Maybe I'm not reproducing this correctly, but in my case it emits the
> > same error even after removing the ent5 line.
> 
> If removing ent5 isn't enough, try removing more entries. You can also
> remove all of the definitions including the [ … ] parenthesis, leaving you
> with just
> 
> …
> <!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd">
> 
> <refentry lang="en">
> …
> 
> At this point C-c C-f should work at the latest.

I cannot seem to make it work here whatever I do, sorry.

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz Bunkus]

Hey,

> I cannot seem to make it work here whatever I do, sorry.

Maybe you're doing something slightly different than I am. I've created a
short screencast showing both nxml-finish-element not working and working
after the entity definitions have been removed:

https://www.bunkus.org/misc/nxml-mode-bug-32823-01.webm

Maybe that'll help.

Kind regards,
mosu

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Eli Zaretskii]

> From: Moritz Bunkus <moritz@bunkus.org>
> Cc: 32823@debbugs.gnu.org
> Date: Mon, 24 Sep 2018 19:51:21 +0200
> 
> > I cannot seem to make it work here whatever I do, sorry.
> 
> Maybe you're doing something slightly different than I am. I've created a
> short screencast showing both nxml-finish-element not working and working
> after the entity definitions have been removed:
> 
> https://www.bunkus.org/misc/nxml-mode-bug-32823-01.webm
> 
> Maybe that'll help.

It did, thanks.  Turns out I was trying "C-c C-f" on the wrong
refentry line (there are too many of them).

Anyway, it sounds like the problem is with using syntax-ppss: its 8th
element is nil in the "good" invocation and non-nil in the "bad" one.
syntax-ppss is invoked on the closing bracket of this line:

  <refentry lang="en">

Maybe the syntax table used by nxml-mode (which it inherits from
sgml-mode) needs some augmentation, I'm not sure.  Hopefully, Stefan
could help.

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[npostavs]

merge 18871 32823
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> Maybe you're doing something slightly different than I am. I've created a
>> short screencast showing both nxml-finish-element not working and working
>> after the entity definitions have been removed:
>> 
>> https://www.bunkus.org/misc/nxml-mode-bug-32823-01.webm
>> 
>> Maybe that'll help.
>
> It did, thanks.  Turns out I was trying "C-c C-f" on the wrong
> refentry line (there are too many of them).
>
> Anyway, it sounds like the problem is with using syntax-ppss: its 8th
> element is nil in the "good" invocation and non-nil in the "bad" one.

I believe this bug is a dup of #18871, the root of the problem is that
the bounds of the prolog (DTD definition) aren't updated after editing.
Should be fixed by the patch in https://debbugs.gnu.org/18871#30

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz Bunkus]

Hey,

thanks for working on these things. Unfortunately the patch doesn't help me
at all. Syntax highlighting & functionality (e.g. closing tags) is broken
right after loading the XML file, and it stays broken after any type of
change, no matter where that change is made (inside or outside the
prologue).

If you need a sample file, you can use this one:

https://gitlab.com/mbunkus/mkvtoolnix/blob/master/doc/man/mkvmerge.xml

It's one of the files I work on regularly, and it's always been broken.

Like I said, the problem occurs right after loading, to. No modifications
need to be made in order to trigger it. To my uninformed eyes my issue
doesn't look like to be related to the issue mine was merged to.

Kind regards,
mosu

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[npostavs]

# undo mistaken merge
unmerge 32823
quit

Moritz Bunkus <moritz@bunkus.org> writes:

> Like I said, the problem occurs right after loading, to. No modifications
> need to be made in order to trigger it. To my uninformed eyes my issue
> doesn't look like to be related to the issue mine was merged to.

Ah, you're right, it's not like #18871.  But I think it's the same as
one of the edge cases Vincent brought up during Bug#33887, and the
second patch in https://debbugs.gnu.org/33887#73 should fix it:

https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Don-t-sgml-syntax-propertize-inside-XML-prolog.patch;msg=73;att=3;bug=33887

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz Bunkus]

Hey,

> Ah, you're right, it's not like #18871.  But I think it's the same as
> one of the edge cases Vincent brought up during Bug#33887, and the
> second patch in https://debbugs.gnu.org/33887#73 should fix it:

That patch does indeed fix the issue for me. Thanks a lot!

Kind regards,
mosu

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Noam Postavsky]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

Moritz Bunkus <moritz@bunkus.org> writes:

>> Ah, you're right, it's not like #18871.  But I think it's the same as
>> one of the edge cases Vincent brought up during Bug#33887, and the
>> second patch in https://debbugs.gnu.org/33887#73 should fix it:
>
> That patch does indeed fix the issue for me. Thanks a lot!

Updated patch for another edge case (https://debbugs.gnu.org/33887#127).


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3664 bytes --]

From 290bbf4341c58339dafe46ed87b1979115306556 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 (Bug#32823)

* 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)
(nxml-mode-prolog-comment): New tests.
---
 lisp/nxml/nxml-mode.el            | 26 +++++++++++++++++++++++++-
 test/lisp/nxml/nxml-mode-tests.el | 21 +++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index 8da9f5ca28..113b8bb104 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -424,6 +424,30 @@ (defun nxml-parent-document-set (parent-document)
     (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 handle `nxml-prolog-regions'.
+  (when (< start nxml-prolog-end)
+    (catch 'done-prolog
+      (dolist (prolog-elem nxml-prolog-regions)
+        (let ((type (aref prolog-elem 0))
+              (pbeg (aref prolog-elem 1))
+              (pend (aref prolog-elem 2)))
+          (when (eq type 'comment)
+            (put-text-property pbeg (1+ pbeg)
+                               'syntax-table (string-to-syntax "< b"))
+            (put-text-property (1- pend) pend
+                               'syntax-table (string-to-syntax "> b")))
+          (when (> pend end)
+            (throw 'done-prolog t)))))
+    (setq start nxml-prolog-end))
+  (if (>= start end)
+      (goto-char end)
+    (goto-char start)
+    (sgml-syntax-propertize start end)))
+
 (defvar tildify-space-string)
 (defvar tildify-foreach-region-function)
 
@@ -518,7 +542,7 @@ (define-derived-mode nxml-mode text-mode "nXML"
 	(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..70816bb9de 100644
--- a/test/lisp/nxml/nxml-mode-tests.el
+++ b/test/lisp/nxml/nxml-mode-tests.el
@@ -78,5 +78,26 @@ (ert-deftest nxml-mode-font-lock-quotes ()
       (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)))))))
+
+(ert-deftest nxml-mode-prolog-comment ()
+  (with-temp-buffer
+    (insert "<?xml version=\"1.0\" encoding=\"utf-8\"?><!-- comment1 -->
+<t><!-- comment2 --></t><!-- comment3 -->")
+    (nxml-mode)
+    ;; Check that all comments are parsed as comments
+    (goto-char (point-min))
+    (search-forward "comment1")
+    (should (nth 4 (syntax-ppss)))
+    (search-forward "comment2")
+    (should (nth 4 (syntax-ppss)))
+    (search-forward "comment3")))
+
 (provide 'nxml-mode-tests)
 ;;; nxml-mode-tests.el ends here
-- 
2.11.0

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Moritz Bunkus]

Hey,

unfortunately I cannot test your patch. I tried building Emacs git master
with your patch applied, but compilation failed with "Symbol’s function
definition is void: gv-define-simple-setter"; next I tried to build tag
emacs-26.2, but your patch doesn't apply against that version.

I can therefore neither confirm nor deny that the patch works for
me. Please give my usual file a try yourself:

https://gitlab.com/mbunkus/mkvtoolnix/blob/master/doc/man/mkvmerge.xml

Just try to close any open tag (C-c C-f); that never worked for me, and
syntax highlighting was thoroughly confused, too.

Kind regards,
mosu

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Vincent Lefevre]

On 2019-05-29 19:40:40 -0400, Noam Postavsky wrote:
> Moritz Bunkus <moritz@bunkus.org> writes:
> 
> >> Ah, you're right, it's not like #18871.  But I think it's the same as
> >> one of the edge cases Vincent brought up during Bug#33887, and the
> >> second patch in https://debbugs.gnu.org/33887#73 should fix it:
> >
> > That patch does indeed fix the issue for me. Thanks a lot!
> 
> Updated patch for another edge case (https://debbugs.gnu.org/33887#127).

Thanks. It seems that all problems are solved!

On 2019-05-30 15:22:09 +0200, Moritz Bunkus wrote:
> Hey,
> 
> unfortunately I cannot test your patch. I tried building Emacs git master
> with your patch applied, but compilation failed with "Symbol’s function
> definition is void: gv-define-simple-setter"; next I tried to build tag
> emacs-26.2, but your patch doesn't apply against that version.
> 
> I can therefore neither confirm nor deny that the patch works for
> me. Please give my usual file a try yourself:
> 
> https://gitlab.com/mbunkus/mkvtoolnix/blob/master/doc/man/mkvmerge.xml
> 
> Just try to close any open tag (C-c C-f); that never worked for me, and
> syntax highlighting was thoroughly confused, too.

Everything seems fine with all the patches against Debian's version.
But note that this is not a valid XML file.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Vincent Lefevre]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On 2019-05-31 00:08:59 +0200, Vincent Lefevre wrote:
> On 2019-05-29 19:40:40 -0400, Noam Postavsky wrote:
> > Moritz Bunkus <moritz@bunkus.org> writes:
> > 
> > >> Ah, you're right, it's not like #18871.  But I think it's the same as
> > >> one of the edge cases Vincent brought up during Bug#33887, and the
> > >> second patch in https://debbugs.gnu.org/33887#73 should fix it:
> > >
> > > That patch does indeed fix the issue for me. Thanks a lot!
> > 
> > Updated patch for another edge case (https://debbugs.gnu.org/33887#127).
> 
> Thanks. It seems that all problems are solved!

Unfortunately, they aren't. :( Emacs hangs on the attached file.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

[-- Attachment #2: test.xml --]
[-- Type: application/xml, Size: 2315 bytes --]

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Noam Postavsky]

tags 32823 fixed
close 32823 27.1
quit

Vincent Lefevre <vincent@vinc17.net> writes:

> On 2019-05-31 00:08:59 +0200, Vincent Lefevre wrote:
>> On 2019-05-29 19:40:40 -0400, Noam Postavsky wrote:
>> > Moritz Bunkus <moritz@bunkus.org> writes:
>> > 
>> > >> Ah, you're right, it's not like #18871.  But I think it's the same as
>> > >> one of the edge cases Vincent brought up during Bug#33887, and the
>> > >> second patch in https://debbugs.gnu.org/33887#73 should fix it:
>> > >
>> > > That patch does indeed fix the issue for me. Thanks a lot!
>> > 
>> > Updated patch for another edge case (https://debbugs.gnu.org/33887#127).
>> 
>> Thanks. It seems that all problems are solved!
>
> Unfortunately, they aren't. :( Emacs hangs on the attached file.

I can't reproduce this, but there have been a lot of patches flying
around.  So I'm pushing to master and closing this bug which I believe
is now fixed.  If you still get the hanging with latest master, open a
new bug, and we'll see about fixing that too.

d414c93b06 2019-06-04T08:42:50-04:00 "Don't sgml-syntax-propertize-inside XML prolog (Bug#32823)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d414c93b062cc3e245a6db0cb764d354d037bd42

bug#32823: 26.1; nxml-mode: "no matching start-tag" with long entity definition list

[Vincent Lefevre]

On 2019-06-04 08:59:01 -0400, Noam Postavsky wrote:
> I can't reproduce this, but there have been a lot of patches flying
> around.  So I'm pushing to master and closing this bug which I believe
> is now fixed.  If you still get the hanging with latest master, open a
> new bug, and we'll see about fixing that too.

I confirm that master does not hang.

But I get a highlighting bug, which doesn't occurs in the emacs-26
branch with the patches. So, perhaps you have missed some patch.

I'll try to get a simple testcase (the ones I posted until now do not
trigger the bug) and report a new bug for that.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)