Repository navigation
fix(scraping): keep text after removed style/link/meta/noscript tags (#2416) - #2421
Open
zhongxiao-chang wants to merge 1 commit into
Open
zhongxiao-chang wants to merge 1 commit into
zhongxiao-chang wants to merge 1 commit into
Conversation
…nclecode#2416) `LXMLWebScrapingStrategy._scrap` removed <style>, <link>, <meta> and <noscript> elements with `element.getparent().remove(element)`. In lxml the text that follows an element inside its parent lives in the element's `.tail`, and `remove()` discards the tail with the element -- so a paragraph that contains one of those tags lost everything after it, while the crawl still reported success. Wikipedia emits `<link rel="mw:PageProp/Category">` inside paragraphs, so this silently truncated ordinary article text. Switch the loop to `drop_tree()`, which moves the tail onto the previous sibling (or the parent when there is none). The <script> loop directly below already re-attaches `.tail` by hand, so the intent of the surrounding code is unchanged; this loop was the one site that still dropped it. Adds tests/unit/test_removed_tags_keep_tail.py: five parametrised shapes (link/meta/style/noscript inside a sentence, tag as first child) plus neighbouring-inline cases, running fully offline through LXMLWebScrapingStrategy -> DefaultMarkdownGenerator. 6 of the 7 cases fail on develop without this change and all 7 pass with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2416.
LXMLWebScrapingStrategy._scrapremoved<style>,<link>,<meta>and<noscript>elements withelement.getparent().remove(element). In lxml the text that follows an element inside its parent lives in that element's.tail, andremove()throws the tail away with the element. A paragraph containing one of those tags therefore lost everything after the tag, while the scrape still reported success — silent data loss.Wikipedia emits
<link rel="mw:PageProp/Category">inside paragraph text, so ordinary article prose was being truncated.Before (develop
c1a9c8d, offline, no browser):After:
The fix is
remove()->drop_tree(), which moves the tail onto the previous sibling (or onto the parent when there is no previous sibling). The<script>loop a few lines below in the same method already re-attaches.tailby hand before removing, so keeping the following text is clearly the intent of the surrounding code; this loop was the one site that still dropped it.List of files changed and why
crawl4ai/content_scraping_strategy.py— switch the style/link/meta/noscript removal loop todrop_tree()so the element's tail (following text) survives; comment explains why.tests/unit/test_removed_tags_keep_tail.py— new regression tests: five parametrised shapes (link / meta / style / noscript inside a sentence, tag as first child of the paragraph) plus tag-between-two-sentences and tag-between-inline-siblings.How Has This Been Tested?
Fully offline —
LXMLWebScrapingStrategy.scrap()plusDefaultMarkdownGenerator.generate_markdown(), no browser, no network.developc1a9c8d(source reverted, test kept): 6 failed, 1 passed — so the tests really do pin this bug.tests/unitsuite, before vs after the change: the failure set is byte-identical (24 failures, all intest_browser_tls_sandbox_flags.py, the pdf tests and the egress tests). Those are environment reds on this Windows box (playwright browser binaries and outbound network are unavailable here), not caused by this change — hence the "existing unit tests pass locally" box below is left unchecked with that caveat.black --checkclean.content_scraping_strategy.pyas a whole is notblack-clean ondevelopeither (pre-existing trailing whitespace and quote-style deviations elsewhere in the file);black --diffdoes not touch any line in this change, and running the formatter over the whole file would bury the fix in unrelated churn, so I left it alone.Related, for merge order: #2377 touches the same class of bug in the excluded-links branch (
link.drop_tree(), around line 327) and is still open. This loop is a separate site; the two patches do not overlap, so whichever lands first the other still applies cleanly.Checklist:
tests/unitfailures are environment-related (no browser binaries / no outbound network on this machine) and are identical before and after this change.