Fix parsing of srcset with whitespaces#458
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR aims to fix the parsing of srcset attributes containing whitespaces by refining the parsing logic. It introduces a new test module with various srcset examples, exposes the SrcSetItem struct for broader use, and updates the srcset parsing function to correctly handle empty string cases.
- Added tests to validate the parsing of srcset strings with different whitespace characters.
- Made the SrcSetItem struct public to allow external access.
- Adjusted the loop logic in parse_srcset to increment the index when encountering empty tokens.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/html/parse_srcset.rs | New test suite for verifying srcset parsing behavior with whitespace handling. |
| tests/html/mod.rs | Registered the new parse_srcset test module. |
| src/html.rs | Made SrcSetItem public, added constants for whitespace characters, and updated loop logic in parse_srcset. |
src/html.rs
Outdated
|
|
||
| // Skip empty strings | ||
| if partial.is_empty() { | ||
| i += 1; |
There was a problem hiding this comment.
Manually incrementing the loop counter within the empty-string handling block may introduce off-by-one errors. Please review the loop structure to ensure that the counter is incremented consistently, avoiding unintended skips of valid tokens.
snshn
force-pushed
the
srcset-white-space-fix
branch
from
db8972d to
fa4e766
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the issue with parsing srcset attributes that contain whitespaces by improving the parsing logic and adding a dedicated test.
- Added tests in tests/html/parse_srcset.rs to verify correct handling of srcset strings with multiple items and varied whitespace.
- Made the SrcSetItem struct public and introduced constants for ASCII whitespaces to support the improved parser logic.
- Adjusted the loop counter logic in parse_srcset to ensure proper iteration over srcset parts.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/html/parse_srcset.rs | Added tests for multiple srcset items with whitespaces. |
| tests/html/mod.rs | Included the new parse_srcset module in the test module. |
| src/html.rs | Made SrcSetItem public, added whitespace constant, and refactored loop increment in parse_srcset. |
| @@ -394,6 +394,8 @@ pub fn parse_srcset(srcset: &str) -> Vec<SrcSetItem> { | |||
| while i < partials.len() { | |||
| let partial = partials[i]; | |||
|
|
|||
There was a problem hiding this comment.
[nitpick] The repositioning of the loop counter increment may be unclear to readers; consider adding a comment explaining why the increment is done before processing the current partial to aid future maintainers.
| // Increment the loop counter before processing the current partial | |
| // to handle cases where the partial is split and reinserted into the list. |
1 participant
No description provided.