From 1ac26733d80ec095a2960641e4c68e30a886a6ae Mon Sep 17 00:00:00 2001 From: sjhddh Date: Sun, 12 Apr 2026 01:25:45 +0200 Subject: [PATCH] fix: prevent duplicate ID segments in full-path selector generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When generating full-path CSS/XPath selectors, elements with id attributes had their selector appended twice — once in the id branch (line 30) and again unconditionally (line 50). This produced selectors like 'body > #main > #main > #target > #target' instead of the correct 'body > #main > #target'. Move the append into the else branch so it only fires for elements without an id (elements with id already append in the if branch). Includes 2 regression tests. --- scrapling/core/mixins.py | 2 +- tests/parser/test_general.py | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/scrapling/core/mixins.py b/scrapling/core/mixins.py index c2e7420..5cbcba4 100644 --- a/scrapling/core/mixins.py +++ b/scrapling/core/mixins.py @@ -47,7 +47,7 @@ class SelectorsGeneration: if counter[target.tag] > 1: part += f":nth-of-type({counter[target.tag]})" if css else f"[{counter[target.tag]}]" - selectorPath.append(part) + selectorPath.append(part) target = target.parent if target is None or target.tag == "html": return " > ".join(reversed(selectorPath)) if css else "//" + "/".join(reversed(selectorPath)) diff --git a/tests/parser/test_general.py b/tests/parser/test_general.py index 26f6e6c..3961660 100644 --- a/tests/parser/test_general.py +++ b/tests/parser/test_general.py @@ -321,6 +321,45 @@ def test_selectors_generation(page): _traverse(page) +def test_full_path_selector_no_duplicate_ids(): + """Test that full path selectors don't duplicate id segments (regression test)""" + html = '

Hello

' + page = Selector(html) + target = page.css("#target").first + + # CSS full path should not duplicate id selectors + css_full = target.generate_full_css_selector + assert css_full.count("#target") == 1, f"Duplicate #target in CSS full path: {css_full}" + assert css_full.count("#main") == 1, f"Duplicate #main in CSS full path: {css_full}" + + # XPath full path should not duplicate id selectors + xpath_full = target.generate_full_xpath_selector + assert xpath_full.count("@id='target'") == 1, f"Duplicate @id='target' in XPath full path: {xpath_full}" + assert xpath_full.count("@id='main'") == 1, f"Duplicate @id='main' in XPath full path: {xpath_full}" + + # The generated CSS selector should actually select the correct element + result = page.css(css_full) + assert len(result) == 1 + assert result.first.text == "Hello" + + +def test_full_path_selector_mixed_id_and_no_id(): + """Test full path selectors with a mix of elements with and without ids""" + html = '

Text

' + page = Selector(html) + target = page.css("p").first + + css_full = target.generate_full_css_selector + # p has no id, so it should appear as a tag name; div has id + assert "#wrapper" in css_full + assert css_full.count("#wrapper") == 1 + + # Verify the selector works + result = page.css(css_full) + assert len(result) == 1 + assert result.first.text == "Text" + + # Miscellaneous Tests def test_getting_all_text(page): """Test getting all text from the page"""