Skip to content

fix(download): keep a row-less table from crashing markdown conversion - #2187

Merged
jackwener merged 2 commits into
jackwener:mainfrom
Benjamin-eecs:fix/rowless-table-crash
Jul 29, 2026
Merged

fix(download): keep a row-less table from crashing markdown conversion#2187
jackwener merged 2 commits into
jackwener:mainfrom
Benjamin-eecs:fix/rowless-table-crash

Conversation

@Benjamin-eecs

@Benjamin-eecs Benjamin-eecs commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

turndown-plugin-gfm reads table.rows[0].parentNode in isHeadingRow, and it does so from two places: the table rule's filter and the keep filter it installs. A <table> that carries no tr makes rows[0] undefined, so the read throws Cannot read properties of undefined (reading 'parentNode') while turndown is still deciding which rule applies, before either filter can return a verdict. The throw escapes downloadArticle() and the whole export fails with code: UNKNOWN, so one such table costs the reader the entire document.

Rich-text editors produce these tables routinely: WeChat's editor wraps plain blocks in a <table> whose cells are div / section rather than tr, which is what #2154 hit with 34 tables in one article. A bare <table></table> is enough to reproduce it.

Rules registered with addRule are matched ahead of the plugin's own, and that covers both entry points for a different reason each: add unshifts onto the rule array so the new rule is tried before the plugin's table rule, and forNode exhausts that array before it ever consults the keep list. Every other table is untouched: a table with a heading row still converts to GFM, and a table with rows but no heading row is still preserved as raw HTML by the plugin's existing keep rule, byte for byte. Only the crash goes away.

To be explicit about what this does not do, since the report also asked for table text as line-broken markdown: heading-less tables that do have rows never crashed and still emit raw HTML, which is deliberate upstream turndown-plugin-gfm behaviour. Changing that is a rendering decision affecting every caller including zhihu download and web read, so it is not folded in here.

This sits in the shared conversion path, so it also covers zhihu download and web read, where the same malformed table on an arbitrary URL fails the command today.

Related issue: Closes #2154.

The audit check fails on js-yaml advisory GHSA-52cp-r559-cp3m, which reproduces on main itself and is unrelated to this change; no dependency file is touched here. #2186 bumps it.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • I updated tests or docs if needed
  • I included output or screenshots when useful

Documentation (if adding/modifying an adapter)

  • Added doc page under docs/adapters/ (if new adapter)
  • Updated docs/adapters/index.md table (if new adapter)
  • Updated sidebar in docs/.vitepress/config.mts (if new adapter)
  • Updated README.md / README.zh-CN.md when command discoverability changed
  • Used positional args for the command's primary subject unless a named flag is clearly better
  • Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

The reported article, which failed before, now exports:

$ opencli weixin download --url "https://mp.weixin.qq.com/s/1nUI9K0IT8s6NG4sS9rkXw" --output ./out
- title: 五万字实操手册:以物抵债全方位解读(2026年版)
  author: 负险不彬
  publish_time: 2026年6月11日 13:01
  status: success
  size: 386.6 KB

Every shape that reaches the failing read is covered, and each one throws the reported error on main:

$ node -e '<turndown + gfm, no override>'
CRASH empty table                  => Cannot read properties of undefined (reading 'parentNode')
CRASH table, text only, no tr      => Cannot read properties of undefined (reading 'parentNode')
CRASH table > colgroup only        => Cannot read properties of undefined (reading 'parentNode')
CRASH table > div (no tr)          => Cannot read properties of undefined (reading 'parentNode')
CRASH tbody, no tr, with text      => Cannot read properties of undefined (reading 'parentNode')
OK    normal table                 => "| A |\n| --- |\n| 1 |"

With the rule in place the same inputs convert, keeping the text and leaving a real table alone:

OK    empty table                => ""
OK    text only, no tr           => "hello"
OK    colgroup only              => ""
OK    div, no tr                 => "cell text"
OK    tbody, no tr               => "text"
OK    normal table               => "| A |\n| --- |\n| 1 |"

The two new cases in src/download/article-download.test.ts are genuine regression tests: restoring only article-download.ts to main and rerunning fails both with TypeError: Cannot read properties of undefined (reading 'parentNode'), the exact error from the report. The text-preservation case uses a <caption> deliberately, because it is the only child that survives inside a row-less table; any other content is foster-parented out by the HTML parser before turndown sees the tree, which would make the assertion pass without the rule doing anything.

src/download, clis/zhihu, clis/weixin and clis/web pass 273 / 273 together, so the two other consumers of downloadArticle() are covered. npm run typecheck, npm run check:typed-error-lint, npm run check:silent-column-drop and scripts/check-doc-coverage.sh --strict all pass. No command signature, argument, or column changed, so cli-manifest.json is untouched.

@Benjamin-eecs
Benjamin-eecs marked this pull request as ready for review July 27, 2026 10:38
Copilot AI review requested due to automatic review settings July 27, 2026 10:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jackwener
jackwener force-pushed the fix/rowless-table-crash branch from d58e32d to 047a3a0 Compare July 29, 2026 12:05
@jackwener
jackwener merged commit 1363564 into jackwener:main Jul 29, 2026
11 checks passed
@Benjamin-eecs
Benjamin-eecs deleted the fix/rowless-table-crash branch August 3, 2026 13:32
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.

[Bug]: weixin/download crashes on complex tables during Markdown conversion

3 participants