Subject: bug#63342: 28.2; dom-by-class does not handle nodes with multiple classes properly

From: Tim Landscheidt <[email-masked]>
Date: Sun, 07 May 2023 03:35:39 +0000

Package: emacs
Version: 28.2
Tags: patch

dom-by-class's docstring says:

| Return elements in DOM that have a class name that matches regexp MATCH.

However, it does not match its argument against the ele-
ments' class names, but their class attributes.  The class
attribute can be composed of multiple, space-separated class
names.

This means that a node:

| <p class="class1 class12">…

will not get matched by (dom-by-class dom "^class1$").

This can be worked around by using matches à la:

| "\\(?:^\\| \\)class1\\(?:$\\| \\)"

The attached patch fixes this by testing the match for each
class name individually.
diff --git a/lisp/dom.el b/lisp/dom.el
index 3066673954a..314a38f194f 100644
--- a/lisp/dom.el
+++ b/lisp/dom.el
@@ -131,7 +131,12 @@ dom-strings
 
 (defun dom-by-class (dom match)
   "Return elements in DOM that have a class name that matches regexp MATCH."
-  (dom-elements dom 'class match))
+  (dom-search dom
+              (lambda (d)
+                (if-let ((class-attr (dom-attr d 'class)))
+                    (seq-find
+                     (apply-partially #'string-match match)
+                     (split-string class-attr " "))))))
 
 (defun dom-by-style (dom match)
   "Return elements in DOM that have a style that matches regexp MATCH."
diff --git a/test/lisp/dom-tests.el b/test/lisp/dom-tests.el
index abb586435a7..213c367b3d8 100644
--- a/test/lisp/dom-tests.el
+++ b/test/lisp/dom-tests.el
@@ -43,7 +43,16 @@ dom-tests--tree
                       (dom-node "div" '((class . "foo")
                                         (style . "color: red;"))
                                 (dom-node "p" '((id . "bar"))
-                                          "foo"))
+                                          "foo")
+                                (dom-node "p" '((id . "test-case-1")
+                                                (class . "class1"))
+                                          "text1")
+                                (dom-node "p" '((id . "test-case-2")
+                                                (class . "class12"))
+                                          "text2")
+                                (dom-node "p" '((id . "test-case-3")
+                                                (class . "class1 class2"))
+                                          "text3"))
                       (dom-node "div" '((title . "2nd div"))
                                 "bar"))))
 
@@ -105,8 +114,9 @@ dom-tests--tree
 
 (ert-deftest dom-tests-texts ()
   (let ((dom (dom-tests--tree)))
-    (should (equal (dom-texts dom) "Test foo bar"))
-    (should (equal (dom-texts dom ", ") "Test, foo, bar"))))
+    (should (equal (dom-texts dom) "Test foo text1 text2 text3 bar"))
+    (should (equal (dom-texts dom ", ")
+                   "Test, foo, text1, text2, text3, bar"))))
 
 (ert-deftest dom-tests-child-by-tag ()
   (let ((dom (dom-tests--tree)))
@@ -121,13 +131,29 @@ dom-tests--tree
 
 (ert-deftest dom-tests-strings ()
   (let ((dom (dom-tests--tree)))
-    (should (equal (dom-strings dom) '("Test" "foo" "bar")))
+    (should (equal (dom-strings dom)
+                   '("Test" "foo" "text1" "text2" "text3" "bar")))
     (should (equal (dom-strings (dom-children dom)) '("Test")))))
 
 (ert-deftest dom-tests-by-class ()
   (let ((dom (dom-tests--tree)))
     (should (equal (dom-tag (dom-by-class dom "foo")) "div"))
-    (should-not (dom-by-class dom "bar"))))
+    (should-not (dom-by-class dom "bar"))
+    (should (equal (mapcar (lambda (d) (dom-attr d 'id))
+                           (dom-by-class dom "class1"))
+                   '("test-case-1" "test-case-2" "test-case-3")))
+    (should (equal (mapcar (lambda (d) (dom-attr d 'id))
+                           (dom-by-class dom "class1$"))
+                   '("test-case-1" "test-case-3")))
+    (should (equal (mapcar (lambda (d) (dom-attr d 'id))
+                           (dom-by-class dom "^class2"))
+                   '("test-case-3")))
+    ;; Test that workaround still works.
+    (should (equal (mapcar (lambda (d) (dom-attr d 'id))
+                           (dom-by-class
+                            dom
+                            "\\(?:^\\| \\)class1\\(?:$\\| \\)"))
+                   '("test-case-1" "test-case-3")))))
 
 (ert-deftest dom-tests-by-style ()
   (let ((dom (dom-tests--tree)))