Cascadingly capture css @import urls and extracting urls from separate css item - #345
Conversation
@import urls and extracting urls from separate css item@import urls and extracting urls from separate css item
d805226 to
9a2b065
Compare
also add `--max-css-jump` flag to limit the max `@import` jump
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for cascading CSS @import handling by detecting embedded CSS items, extracting their URLs, and controlling import depth.
- Introduced detection of embedded CSS items and jump-count logic
- Hooked into HTML and CSS extraction to enqueue
@importURLs as children - Added
MaxCSSJumpconfiguration and CLI flag to limit import recursion
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/pkg/postprocessor/item.go | Skip depth limit for embedded CSS items when crawling assets |
| internal/pkg/postprocessor/extractor/html.go | Call AddAtImportLinksToItemChild after parsing CSS in HTML <style> blocks |
| internal/pkg/postprocessor/extractor/css_embedded.go | New logic for detecting embedded CSS and handling @import jump limits |
| internal/pkg/postprocessor/assets.go | Added branch for extracting assets from CSS items and capturing @import links |
| internal/pkg/config/config.go | Added MaxCSSJump field to configuration |
| cmd/get.go | Exposed --max-css-jump CLI flag |
Comments suppressed due to low confidence (3)
internal/pkg/postprocessor/extractor/css_embedded.go:1
- There are no unit tests covering the new embedded CSS extractor logic. Please add tests for
IsEmbeddedCSS,GetEmbeddedCSSJump, andAddAtImportLinksToItemChildto validate expected behavior.
package extractor
internal/pkg/postprocessor/item.go:74
- The word "embeded" is misspelled; please correct it to "embedded".
// 1. the item [is not an embeded css item] and [is a child has a depth (without redirections) bigger than 2].
internal/pkg/postprocessor/extractor/css_embedded.go:26
- The variable
cssLoggeris not defined or imported in this file, which will cause a compile error. Please import the appropriate logging package or reference the correct logger instance.
cssLogger.Warn("item is not an embedded CSS, returning 0 @import jump", "func", "GetEmbeddedCSSJump", "item_id", item.GetShortID())
|
The biggest issue with this CSS parser at the moment is that it doesn't support nested CSS yet, which is widely used today. Writing a fix for upstream might be time-consuming, so the mitigation is to add "if parsing fails, fall back to the regex based extractor we used before" logic. I'll to add the logic hours later... |
| if p.Err() != nil && !errors.Is(p.Err(), io.EOF) { | ||
| parseErr = p.Err() | ||
| cssLogger.Error("error parsing CSS", "error", parseErr, "inline", inline) | ||
| cssLogger.Error("error parsing CSS", "inline", inline, "error", parseErr) |
There was a problem hiding this comment.
The parseErr include newlines, like this:
unexpected token '[' in declaration on line 5 and column 5
5: [aria-hidden="true"] {
^
So I put the err parameter at the end to make the terminal log look nicer.
NGTmeaty
left a comment
There was a problem hiding this comment.
This looks great! And thanks a ton for adding the regex fallback.
@importjumps, capturing@importURLs cascadingly.@importjumps, added the--max-css-jumpoption to limit it.tdewolff/parseparser currently does not support nested CSS syntax, the previous regex extractor was added back as a fallback whentdewolff/parsefails. (Also improved the previous CSS regex; it should now produce less non-existent links.)close: #323