Subject: Re: bug#34685: 26.1; function nnrss-get-namespace-prefix always returns nil

From: Benjamin Riefenstahl <[email-masked]>
Date: Mon, 09 Aug 2021 18:30:01 +0200

Hi Lars,

> Benjamin Riefenstahl <[email-masked]> writes:
>> LS09LT0tPQpDb250ZW50LVR5cGU6IHRleHQvcGxhaW4KCkhpIExhcnMsCgpMYXJzIEluZ2Vicmln
>> dHNlbiB3cml0ZXM6Cj4gSSd2ZSBub3cgZml4ZWQgdGhpcyBtb3JlIGdlbmVyYWxseSBieSBzZWFy

Lars Ingebrigtsen writes:
> Your message arrived kinda destroyed, but I used my decoder ring:

Sorry about that.  My laptop does not have email setup correctly, that
made things complicated in this case.

>> +(defconst test-nnrss-xml
>> +  '((rss
>> +     ((version . "2.0")
>> +      (xmlns:dc . "http://purl.org/dc/elements/1.1/"))
>> +     (channel
>> +      ((xmlns:content . "http://purl.org/rss/1.0/modules/content/"))))))

> The problem here is that this test XML isn't a valid DOM -- it's a
> list of DOMs.  A valid DOM would be

I originally just copied this from the output of the debugger, so this
is what nnrss-get-namespace-prefix gets passed.  I just checked, the
structure is the result of xml-parse-region, is that supposed to create
something that matches the use of dom-search?  When I try it on some
test XML it does create this same structure again.

Test code:

    (with-temp-buffer
      (insert "<rss xmlns:content='http://purl.org/rss/1.0/modules/content/'>
                 <channel />
               </rss>")
      (xml-parse-region (point-min) (point-max)))

> So if EL is a list of nodes, then it's the caller of
> nnrss-get-namespace-prefix here that should be adjusted to pass in the
> car of the list instead?  Could you try that and see whether that
> works?

What is passed to nnrss-get-namespace-prefix is also used in other
places and the calling function is a bit complicated so I'd rather not
change it.  I attach another change in nnrss-get-namespace-prefix
function instead.

I note that your previous work on this bug is the first time that
nnrss.el uses functions from dom.el.  It maybe should use it more, but
that is a project for another day, I would say.

> (And send a new patch; I've applied your test (adjusted).)

See below.

Thanks for your time,
benny

>From f7f9bc841aa9a4b6fd5a3d8dcffd259f01b10591 Mon Sep 17 00:00:00 2001
From: Benjamin Riefenstahl <[email-masked]>
Date: Mon, 9 Aug 2021 18:06:31 +0200
Subject: [PATCH] Fix nnrss-get-namespace-prefix (bug#34685)

nnrss-get-namespace-prefix gets passed the result of xml-parse-region
and that is not quite what dom-search expects.

* lisp/gnus/nnrss.el (nnrss-get-namespace-prefix): Use the car of
parameter el to match what dom-search expects.
* test/lisp/gnus/nnrss-tests.el (test-nnrss-xml): Adjust to what
xml-parse-region produces.
---
 lisp/gnus/nnrss.el            |  7 ++++---
 test/lisp/gnus/nnrss-tests.el | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/gnus/nnrss.el b/lisp/gnus/nnrss.el
index 0f12ee0e9d..97c9f18a60 100644
--- a/lisp/gnus/nnrss.el
+++ b/lisp/gnus/nnrss.el
@@ -954,9 +954,10 @@ nnrss-get-namespace-prefix
   "Given EL (containing a parsed element) and URI (containing a string
 that gives the URI for which you want to retrieve the namespace
 prefix), return the prefix."
-  (let* ((prefix (car (rassoc uri (dom-attributes
-				   (dom-search
-				    el
+  (let* ((dom (car el))
+         (prefix (car (rassoc uri (dom-attributes
+			           (dom-search
+				    dom
 				    (lambda (node)
 				      (rassoc uri (dom-attributes node))))))))
 	 (nslist (if prefix
diff --git a/test/lisp/gnus/nnrss-tests.el b/test/lisp/gnus/nnrss-tests.el
index 01b374a2f6..92b7dacf18 100644
--- a/test/lisp/gnus/nnrss-tests.el
+++ b/test/lisp/gnus/nnrss-tests.el
@@ -27,11 +27,11 @@ test-nnrss-normalize
                  "Fri, 17 Sep 2004 05:09:49 +0000")))
 
 (defconst test-nnrss-xml
-  '(rss
-    ((version . "2.0")
-     (xmlns:dc . "http://purl.org/dc/elements/1.1/"))
-    (channel
-     ((xmlns:content . "http://purl.org/rss/1.0/modules/content/")))))
+  '((rss
+     ((version . "2.0")
+      (xmlns:dc . "http://purl.org/dc/elements/1.1/"))
+     (channel
+      ((xmlns:content . "http://purl.org/rss/1.0/modules/content/"))))))
 
 (ert-deftest test-nnrss-namespace-top ()
   (should (equal (nnrss-get-namespace-prefix
-- 
2.20.1