Skip to content

fix: prefer data-src over placeholder data URI in img src - #2417

Merged
afourney merged 3 commits into
microsoft:mainfrom
macjayz:fix-lazy-load-image-src
Sep 10, 2026
Merged

fix: prefer data-src over placeholder data URI in img src#2417
afourney merged 3 commits into
microsoft:mainfrom
macjayz:fix-lazy-load-image-src

Conversation

@macjayz

Copy link
Copy Markdown
Contributor

convert_img falls back to data-src only when src is missing or empty. Lazy-loaded images commonly leave a placeholder data: URI in src and the real URL in data-src, so that case still lost the real image. This treats a data: URI in src the same as empty when data-src is present.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally (Arch Linux, Python 3.14.7, markitdown from source at b6e8bbd).

Reproduced before the fix. Applying only this PR's test file to main without the source change fails 1 test (test_img_prefers_data_src_over_placeholder_data_uri), so the placeholder data URI does currently win over the real URL in data-src.

After the fix: all 10 tests in test_html_converter.py pass. Wider selection (-k "html or markdownify", ignoring test_outlook_msg_ansi.py which fails to collect here for an unrelated missing module): 24 passed / 2 failed with the change vs 20 passed / 2 failed on main. The 2 failures are the test_docintel_* tests, failing identically before and after, so they are pre-existing in my environment. No regressions.

Small note, not a blocker: src[:5].lower() == "data:" matches the scheme without requiring a colon-normalised parse. It is fine for real-world markup, and DATA:/Data: are covered by the .lower().

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation matches the stated behavior and has adequate regression coverage.

Pull request overview

Updates HTML image conversion to prefer lazy-loaded image URLs over placeholder data URIs.

Changes:

  • Selects data-src when src is empty or a data URI.
  • Adds regression and fallback tests.
File summaries
File Description
_markdownify.py Improves lazy-loaded image source selection.
test_html_converter.py Tests image source precedence and fallback behavior.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation matches the intended behavior and covers the relevant precedence cases.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@afourney
afourney merged commit b59e064 into microsoft:main Sep 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants