fix: relative base URL double-prefix and skipped inline styles - #1826
Conversation
Two more relative-base bugs in the same transformer (both masked by absolute bases, which self-guard via isAbsoluteUrl after one prepend): - VML double-prefix: a v:image/v:fill src inside an MSO comment is rewritten by both rewriteVMLs and rewriteMsoComments' generic src pass, so a relative base is prepended twice (/images//images/…). Guard the MSO/VML passes with startsWith(url) so an already-prefixed src is left alone. - Inline style url() skipped: the inline-style url() rewrite sat behind the `tags` filter, so `tags: ['img']` skipped a background image on a non-listed tag (e.g. <td>). Move it out of the tag gate — it is governed by `inlineCss`, like <style> tag handling. Add regression tests for both (relative base).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe transformer now rewrites inline CSS URLs independently of tag filtering. VML and MSO comment processing avoids duplicate relative base URLs. Regression tests cover restricted tags and ChangesURL rewriting behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/transformers/base.ts`:
- Around line 262-267: Update the URL-prefix checks in the VML rewrite
callbacks, including the v:fill and v:image handling, so a source is skipped
only when it matches the configured base path at a path boundary rather than
merely sharing its string prefix; preserve absolute-URL handling and support
bases without a trailing slash. Add regression coverage for both VML elements
using a base such as /images and sources under /images2, ensuring they are
rewritten.
🪄 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 Plus
Run ID: 9d76d8e8-0b0a-4214-a932-0e148fb093f1
📒 Files selected for processing (2)
src/tests/transformers/base.test.tssrc/transformers/base.ts
The `startsWith(url)` skip guard false-matched when the base had no trailing slash: base `/images` would skip (not rewrite) a source under `/images2` because it merely shares the string prefix. Require a path boundary (exact match or a following `/`) via `isUnderBase`, applied to the v:image/v:fill and MSO-comment passes. Add regression coverage for both VML elements with base `/images` and sources under `/images2`.
Two more relative-base bugs in the base URL transformer, both surfaced with a relative
url.base(e.g./images/) and masked by absolute bases — an absolute base becomes absolute after one prepend, soisAbsoluteUrlself-guards. Follow-up to #1825 (the infinite-loop fix), same file.1. VML
srcdouble-prefixed inside MSO commentsA
v:image/v:fillsrcinside an MSO conditional comment is rewritten by bothrewriteVMLsandrewriteMsoComments' genericsrc=pass. With a relative base neither pass sees the value as absolute, so it's prepended twice:Fix: guard the MSO/VML passes with
startsWith(url)so an already-prefixedsrc(from the first pass) is left alone. Absolute bases keep self-guarding viaisAbsoluteUrl.2. Inline
styleurl()skipped whentagsis restrictedThe inline-
styleurl()rewrite sat inside thetags-filter gate, sotags: ['img']skipped a background image on any non-listed tag:The
tagsfilter is meant to restrict source attributes (src/href/…), not CSSurl()rewriting — which is governed byinlineCss(mirroring how<style>tags are handled). Fix: move the inline-style rewrite out of the tag gate.Tests
Added regression tests (relative base):
v:fill/v:imagein MSO comments prefixed exactly once (no/images//images/), and inlinestyleurl()still rewritten undertags: ['img']. Full suite green (1772), lint clean.Summary by CodeRabbit