`prev()` function fails when non-element nodes are present (#30117)

The `prev()` function doesn't take nodes other than elements into account. Also we could simplify things a lot using the `previousElementSibling` property. This property isn't fully supported in IE, it only works for elements, but since the `element` variable is an element, we can safely use it here.

I've also added an additional test.

I don't think we had this issue in v4, since we relied on jQuery back then.

Ref. https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode/nextElementSibling

2 files changed, 20 insertions, 7 deletions

diff --git a/js/src/dom/selector-engine.js b/js/src/dom/selector-engine.js
index d66d8acfc8..c9d25f68cb 100644
--- a/js/src/dom/selector-engine.js
+++ b/js/src/dom/selector-engine.js
@@ -56,19 +56,17 @@ const SelectorEngine = {
   },
 
   prev(element, selector) {
-    const siblings = []
+    let previous = element.previousElementSibling
 
-    let previous = element.previousSibling
-
-    while (previous && previous.nodeType === Node.ELEMENT_NODE && previous.nodeType !== NODE_TEXT) {
+    while (previous) {
       if (this.matches(previous, selector)) {
-        siblings.push(previous)
+        return [previous]
       }
 
-      previous = previous.previousSibling
+      previous = previous.previousElementSibling
     }
 
-    return siblings
+    return []
   }
 }
 
diff --git a/js/tests/unit/dom/selector-engine.spec.js b/js/tests/unit/dom/selector-engine.spec.js
index e13438e6fd..e140c6a3e8 100644
--- a/js/tests/unit/dom/selector-engine.spec.js
+++ b/js/tests/unit/dom/selector-engine.spec.js
@@ -110,6 +110,21 @@ describe('SelectorEngine', () => {
 
       expect(SelectorEngine.prev(btn, '.test')).toEqual([divTest])
     })
+
+    it('should return previous element with comments or text nodes between', () => {
+      fixtureEl.innerHTML = [
+        '<div class="test"></div>',
+        '<div class="test"></div>',
+        '<!-- Comment-->',
+        'Text',
+        '<button class="btn"></button>'
+      ].join('')
+
+      const btn = fixtureEl.querySelector('.btn')
+      const divTest = fixtureEl.querySelectorAll('.test')[1]
+
+      expect(SelectorEngine.prev(btn, '.test')).toEqual([divTest])
+    })
   })
 })