From 1ac26733d80ec095a2960641e4c68e30a886a6ae Mon Sep 17 00:00:00 2001 From: sjhddh Date: Sun, 12 Apr 2026 01:25:45 +0200 Subject: [PATCH 1/2] 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""" From 273c8c2fa02db3003db604c8b21447574440d3c0 Mon Sep 17 00:00:00 2001 From: sjhddh Date: Sun, 12 Apr 2026 09:46:55 +0200 Subject: [PATCH 2/2] fix: emit valid XPath node test for ID elements in full-path mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: In full-path XPath generation, elements with IDs were producing bare predicates like `[@id='x']` which creates invalid XPath steps like `//body/[@id='main']`. Now emits `*[@id='x']` for full-path mode (e.g. `//body/*[@id='main']/*[@id='target']`). Short-path XPath mode unchanged — still uses `//*[@id='x']` prefix. Also added XPath evaluation assertion to the regression test to verify the generated selector actually selects the correct element. Co-Authored-By: Claude Opus 4.6 --- scrapling/core/mixins.py | 7 ++++++- tests/parser/test_general.py | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/scrapling/core/mixins.py b/scrapling/core/mixins.py index 5cbcba4..868d620 100644 --- a/scrapling/core/mixins.py +++ b/scrapling/core/mixins.py @@ -26,7 +26,12 @@ class SelectorsGeneration: if target.parent: if target.attrib.get("id"): # id is enough - part = f"#{target.attrib['id']}" if css else f"[@id='{target.attrib['id']}']" + if css: + part = f"#{target.attrib['id']}" + elif full_path: + part = f"*[@id='{target.attrib['id']}']" + else: + part = f"[@id='{target.attrib['id']}']" selectorPath.append(part) if not full_path: 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 3961660..293e93e 100644 --- a/tests/parser/test_general.py +++ b/tests/parser/test_general.py @@ -342,6 +342,11 @@ def test_full_path_selector_no_duplicate_ids(): assert len(result) == 1 assert result.first.text == "Hello" + # The generated XPath selector should also select the correct element + result = page.xpath(xpath_full) + assert len(result) == 1, f"XPath '{xpath_full}' selected {len(result)} elements, expected 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"""