Normalize problematic EPUB image sizing#41
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe EPUB optimizer adds shared helpers to remove image dimensions and sizing declarations, rewrite raster references, and apply the same normalization to namespaced and namespace-less XHTML images. ChangesImage sizing normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crosspoint_reader/optimizer.py (2)
316-325: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBypass string manipulation when the style attribute doesn't need modification.
Just like the logic in
_fix_css, returningm.group(0)early when no declarations are problematic prevents unnecessary string rewriting and preserves the exact original whitespace in the document.♻️ Proposed refactor
def _fix_img_style_text(tag): def repl(m): quote = m.group(1) style = m.group(2) - fixed, _ = _fix_img_style_attr(style) + fixed, modified = _fix_img_style_attr(style) + if not modified: + return m.group(0) if fixed: return ' style=%s%s%s' % (quote, fixed, quote) return ''🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crosspoint_reader/optimizer.py` around lines 316 - 325, The _fix_img_style_text replacement currently rewrites valid style attributes and can alter their original formatting. Update repl to return m.group(0) when _fix_img_style_attr reports no problematic declarations, while preserving the existing rewritten output only when fixed contains a correction.
391-394: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the bitwise OR assignment
|=for safer boolean accumulation.While
modified = _fix_img_element(img) or modifiedis technically correct because Python evaluates the left operand first (ensuring the function runs), it relies on a subtle ordering detail that is easily broken during future refactoring. For example, flipping it tomodified or _fix_img_element(...)would introduce a severe short-circuit bug where images stop being processed. Using|=is more idiomatic and eliminates this risk entirely.♻️ Proposed refactor
- modified = _fix_img_element(img) or modified + modified |= _fix_img_element(img) # also catch namespace-less <img> (malformed docs) for img in root.iter('img'): - modified = _fix_img_element(img) or modified + modified |= _fix_img_element(img)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crosspoint_reader/optimizer.py` around lines 391 - 394, Replace the boolean accumulation assignments in the image-processing loops around _fix_img_element with |=, preserving the call for every image while accumulating whether any element was modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crosspoint_reader/optimizer.py`:
- Around line 316-325: The _fix_img_style_text replacement currently rewrites
valid style attributes and can alter their original formatting. Update repl to
return m.group(0) when _fix_img_style_attr reports no problematic declarations,
while preserving the existing rewritten output only when fixed contains a
correction.
- Around line 391-394: Replace the boolean accumulation assignments in the
image-processing loops around _fix_img_element with |=, preserving the call for
every image while accumulating whether any element was modified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a383140-afb4-4203-a89a-a20fcc188098
📒 Files selected for processing (1)
crosspoint_reader/optimizer.py
thiagokokada
force-pushed
the
fix-crosspoint-image-sizing
branch
from
dd08ccd to
a0d24fe
Compare
thiagokokada
force-pushed
the
fix-crosspoint-image-sizing
branch
from
a0d24fe to
ef124ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crosspoint_reader/optimizer.py`:
- Line 324: Update the style-attribute substitution in the optimizer’s re.sub
call to include re.DOTALL alongside re.IGNORECASE, so the existing non-greedy
capture matches style values spanning newlines while preserving current
normalization behavior.
- Around line 285-310: Update _filter_style_declarations to remove matching
declarations without naively splitting style by semicolons; use a regex-based
replacement that preserves semicolons and formatting inside quoted strings or
data URIs. Keep predicate-based filtering for width and height via
_is_problem_img_decl and preserve the function’s return value and modified flag
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19509cda-073b-470a-9baa-be051bcd4524
⛔ Files ignored due to path filters (1)
crosspoint_reader/__pycache__/optimizer.cpython-313.pycis excluded by!**/*.pyc
📒 Files selected for processing (1)
crosspoint_reader/optimizer.py
1 participant
Summary
width/heightattributes from<img>tags as before.width/heightdeclarations from<img style="...">.Problem
Some EPUBs include inline image sizing that overrides CrossPoint’s defensive responsive image CSS.
Example:
The optimizer already injects:
But inline width / height declarations have higher precedence, so values like
height: 98vhcan prevent optimized images from rendering at the expected size.Fix
When rewriting XHTML, the optimizer now removes any inline width or height declaration from
<img style="...">, while preserving unrelated style declarations.This mirrors the existing behavior for
width=andheight=attributes and keeps the change scoped to inline image markup.Validation
Notes