Optimized out id() and ownerDoc() calls

Author: jhy

CHANGES.md

@@ -1,5 +1,12 @@
 # jsoup Changelog
 
+## 1.19.2 (PENDING)
+
+### Changes
+
+### Improvements
+* `Element.cssSelector()` will prefer to return shorter selectors by using ancestor IDs when available and unique. E.g. `#id > div > p` instead of  `html > body > div > div > p` [#2283](https://github.com/jhy/jsoup/pull/2283).
+
 ## 1.19.1 (2025-03-04)
 
 ### Changes

src/main/java/org/jsoup/nodes/Element.java

@@ -937,47 +937,47 @@ public Element wrap(String html) {
         return (Element) super.wrap(html);
     }
 
-    private String idAsCssSelector() {
-        String result = "";
-
-        if (!id().isEmpty()) {
-            // prefer to return the ID - but check that it's actually unique first!
-            String idSel = "#" + escapeCssIdentifier(id());
-            Document doc = ownerDocument();
-            if (doc != null) {
-                Elements els = doc.select(idSel);
-                if (els.size() == 1 && els.get(0) == this) // otherwise, continue to the nth-child impl
-                    result = idSel;
+    /**
+     Gets an #id selector for this element, if it has a unique ID. Otherwise, returns an empty string.
+
+     @param ownerDoc the document that owns this element, if there is one
+     */
+    private String uniqueIdSelector(@Nullable Document ownerDoc) {
+        String id = id();
+        if (!id.isEmpty()) { // check if the ID is unique and matches this
+            String idSel = "#" + escapeCssIdentifier(id);
+            if (ownerDoc != null) {
+                Elements els = ownerDoc.select(idSel);
+                if (els.size() == 1 && els.get(0) == this) return idSel;
             } else {
-                result = idSel; // no ownerdoc, return the ID selector
+                return idSel;
             }
         }
-
-        return result;
+        return EmptyString;
     }
 
     /**
-     * Get a CSS selector that will uniquely select this element.
-     * <p>
-     * If the element has an ID, returns #id;
-     * otherwise returns the parent (if any) CSS selector, followed by {@literal '>'},
-     * followed by a unique selector for the element (tag.class.class:nth-child(n)).
-     * </p>
-     *
-     * @return the CSS Path that can be used to retrieve the element in a selector.
+     Get a CSS selector that will uniquely select this element.
+     <p>
+     If the element has an ID, returns #id; otherwise returns the parent (if any) CSS selector, followed by
+     {@literal '>'}, followed by a unique selector for the element (tag.class.class:nth-child(n)).
+     </p>
+
+     @return the CSS Path that can be used to retrieve the element in a selector.
      */
     public String cssSelector() {
-        String idAsCssSelector = idAsCssSelector();
-        if (!idAsCssSelector.isEmpty())
-            return idAsCssSelector;
+        Document ownerDoc = ownerDocument();
+        String idSel = uniqueIdSelector(ownerDoc);
+        if (!idSel.isEmpty()) return idSel;
 
+        // No unique ID, work up the parent stack and find either a unique ID to hang from, or just a GP > Parent > Child chain
         StringBuilder selector = StringUtil.borrowBuilder();
         Element el = this;
         while (el != null && !(el instanceof Document)) {
-            idAsCssSelector = el.idAsCssSelector();
-            if (!idAsCssSelector.isEmpty()) {
-                selector.insert(0, idAsCssSelector);
-                break;
+            idSel = el.uniqueIdSelector(ownerDoc);
+            if (!idSel.isEmpty()) {
+                selector.insert(0, idSel);
+                break; // found a unique ID to use as ancestor; stop
             }
             selector.insert(0, el.cssSelectorComponent());
             el = el.parent();

src/test/java/org/jsoup/nodes/ElementTest.java

@@ -2594,13 +2594,24 @@ void prettySerializationRoundTrips(Document.OutputSettings settings) {
 
     @Test void cssSelectorParentWithId() {
         // https://github.com/jhy/jsoup/issues/2282
-        Document doc = Jsoup.parse("<div><div id=\"id1\"><p>A</p></div><div><p>B</p></div><div class=\"c1 c2\"><p>C</p></div></div>");
-        Element divA = doc.select("div div p").get(0);
-        Element divB = doc.select("div div p").get(1);
-        Element divC = doc.select("div div p").get(2);
-        assertEquals("#id1 > p", divA.cssSelector());
-        assertEquals("html > body > div > div:nth-child(2) > p", divB.cssSelector());
-        assertEquals("html > body > div > div.c1.c2 > p", divC.cssSelector());
+        Document doc = Jsoup.parse("<div><div id=id1><p>A</p></div><div><p>B</p></div><div class='c1 c2'><p>C</p></div></div>");
+        Elements els = doc.select("p");
+        Element pA = els.get(0);
+        Element pB = els.get(1);
+        Element pC = els.get(2);
+        assertEquals("#id1 > p", pA.cssSelector());
+        assertEquals("html > body > div > div:nth-child(2) > p", pB.cssSelector());
+        assertEquals("html > body > div > div.c1.c2 > p", pC.cssSelector());
+    }
+
+    @Test void cssSelectorWithNonUniqueId() {
+        Document doc = Jsoup.parse("<main id=out><div><div id=in>One</div><div id=in>Two</div></div></main>");
+        Element two = doc.expectFirst("div:containsOwn(Two)");
+        String selector = two.cssSelector();
+        assertEquals("#out > div > div:nth-child(2)", selector);
+        Elements found = doc.select(selector);
+        assertEquals(1, found.size());
+        assertEquals(two, found.first());
     }
 
     @Test void cssSelectorDoesntStackOverflow() {