-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: links not being scraped #552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change updates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Scraper
participant DOM
User->>Scraper: Request element value extraction
Scraper->>DOM: Inspect element attributes
alt Attribute is 'href' and element is not <a>
Scraper->>DOM: Find parent <a> element
alt Parent <a> exists
Scraper->>DOM: Get href from parent <a>
Scraper->>User: Return absolute URL
else No parent <a>
Scraper->>User: Return attribute or data attribute value
end
else Other attributes or cases
Scraper->>DOM: Extract as per previous logic
Scraper->>User: Return value
end
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
maxun-core/src/browserSide/scraper.js (1)
528-529: Consider adding more fallback options for baseURL.While using
element.ownerDocument?.location?.href || window.location.originworks in most cases, some complex DOM structures might benefit from additional fallbacks.-const baseURL = element.ownerDocument?.location?.href || window.location.origin; +const baseURL = element.ownerDocument?.location?.href || + element.ownerDocument?.baseURI || + document.baseURI || + window.location.origin;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
maxun-core/src/browserSide/scraper.js(1 hunks)server/src/workflow-management/selector.ts(0 hunks)
💤 Files with no reviewable changes (1)
- server/src/workflow-management/selector.ts
🔇 Additional comments (2)
maxun-core/src/browserSide/scraper.js (2)
544-556: Good enhancement for href extraction in non-anchor elements!This new functionality improves URL extraction accuracy by checking for parent
<a>elements when the current element isn't an anchor. This is particularly useful for situations where clickable elements (like images, buttons, or spans) are nested inside anchor tags - a common pattern in modern web development.
526-581: The overall implementation of extractValue is robust and well structured.The function handles various cases thoroughly:
- Checks for shadow root content
- Processes innerText and innerHTML attributes
- Special handling for src/href attributes including parent element checking
- Fallback to data-attributes and background images
- Proper URL resolution using the context-aware base URL
The try-catch blocks for URL construction (lines 549-553 and 573-577) provide good error handling for malformed URLs.
What this PR does?
Fixes the issue of links not being scraped at certain sites.
Closes #548
Summary by CodeRabbit
Bug Fixes
Chores