-
Notifications
You must be signed in to change notification settings - Fork 7
fix(editor): keep markdown tables intact through the publish editor #1482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| import { vi } from "vitest"; | ||
|
|
||
| vi.mock("@/features/tiptap-editor/extensions", () => ({ | ||
| HIVE_POST_PURE_REGEX: /$a^/, | ||
| LOOM_REGEX: /$a^/, | ||
| TAG_MENTION_PURE_REGEX: /$a^/, | ||
|
qodo-code-review[bot] marked this conversation as resolved.
|
||
| USER_MENTION_PURE_REGEX: /$a^/, | ||
| YOUTUBE_REGEX: /$a^/ | ||
| })); | ||
|
|
||
| import { Editor } from "@tiptap/core"; | ||
| import StarterKit from "@tiptap/starter-kit"; | ||
| import Table from "@tiptap/extension-table"; | ||
| import TableCell from "@tiptap/extension-table-cell"; | ||
| import TableHeader from "@tiptap/extension-table-header"; | ||
| import TableRow from "@tiptap/extension-table-row"; | ||
| import { simpleMarkdownToHTML } from "@ecency/render-helper"; | ||
|
|
||
| import { markdownToHtml } from "@/features/tiptap-editor/functions/markdown-to-html"; | ||
| import { parseAllExtensionsToDoc } from "@/features/tiptap-editor/functions/parse-all-extensions-to-doc"; | ||
|
|
||
| const TABLE_MARKDOWN = [ | ||
| "| Date | Account | Amount |", | ||
| "| --- | --- | --- |", | ||
| "| 2023-01-06 | valueplan | 52,239.55 |", | ||
| "| 2020-04-03 | ecency | 941.94 |" | ||
| ].join("\n"); | ||
|
|
||
| /** | ||
| * Mirrors what the publish editor actually does: paste plain text (converted by | ||
| * the clipboard strategy), then serialize the document back to markdown the way | ||
| * `use-publish-editor` does on every update. | ||
| */ | ||
| function pasteThenSerialize(markdown: string) { | ||
| const editor = new Editor({ | ||
|
qodo-code-review[bot] marked this conversation as resolved.
Outdated
|
||
| extensions: [StarterKit, Table, TableRow, TableCell, TableHeader], | ||
| content: "<p></p>" | ||
| }); | ||
| try { | ||
| editor.chain().insertContent(parseAllExtensionsToDoc(simpleMarkdownToHTML(markdown))).run(); | ||
| return markdownToHtml(editor.getHTML()); | ||
| } finally { | ||
| editor.destroy(); | ||
| } | ||
| } | ||
|
|
||
| describe("markdown table round-trip through the publish editor", () => { | ||
| // Regression: Turndown was built with only the `strikethrough` GFM plugin, so | ||
| // it had no rule for <table> and flattened every pasted table into loose text | ||
| // on the first serialization pass. | ||
| it("keeps a pasted table a table", () => { | ||
| const result = pasteThenSerialize(TABLE_MARKDOWN); | ||
|
|
||
| expect(result).toContain("| Date | Account | Amount |"); | ||
| expect(result).toContain("| 2023-01-06 | valueplan | 52,239.55 |"); | ||
| expect(result).toContain("| 2020-04-03 | ecency | 941.94 |"); | ||
| }); | ||
|
|
||
| it("keeps every row on its own line with a delimiter row", () => { | ||
| const lines = pasteThenSerialize(TABLE_MARKDOWN) | ||
| .split("\n") | ||
| .map((line) => line.trim()) | ||
| .filter((line) => line.startsWith("|")); | ||
|
|
||
| // header + delimiter + 2 body rows | ||
| expect(lines).toHaveLength(4); | ||
| expect(lines[1]).toMatch(/^\|[\s-|:]+\|$/); | ||
| lines.forEach((line) => expect(line.split("|")).toHaveLength(5)); | ||
|
qodo-code-review[bot] marked this conversation as resolved.
|
||
| }); | ||
|
|
||
| // Regression: TipTap emits <colgroup> before <tbody> and keeps the header | ||
| // cells as <th> inside that <tbody>, which made the GFM rule miss the heading | ||
| // row and prepend an empty one ("| | | |") above the real headers. | ||
| it("uses the real header row rather than prepending an empty one", () => { | ||
| const lines = pasteThenSerialize(TABLE_MARKDOWN) | ||
| .split("\n") | ||
| .map((line) => line.trim()) | ||
| .filter((line) => line.startsWith("|")); | ||
|
|
||
| expect(lines[0]).toContain("Date"); | ||
| expect(lines[0]).toContain("Account"); | ||
| expect(lines[0]).not.toMatch(/^\|(\s*\|)+$/); | ||
| }); | ||
|
|
||
| it("does not flatten cells into loose paragraphs", () => { | ||
| const result = pasteThenSerialize(TABLE_MARKDOWN); | ||
|
|
||
| // the pre-fix output was "Date\n\nAccount\n\nAmount\n\n2023-01-06\n\n..." | ||
| expect(result).not.toMatch(/^Date\s*$/m); | ||
| expect(result).not.toMatch(/^valueplan\s*$/m); | ||
| }); | ||
|
|
||
| it("serializes a table built with the editor's own insertTable command", () => { | ||
| const editor = new Editor({ | ||
| extensions: [StarterKit, Table, TableRow, TableCell, TableHeader], | ||
| content: "<p></p>" | ||
| }); | ||
| try { | ||
| editor.chain().focus().insertTable({ rows: 2, cols: 2, withHeaderRow: true }).run(); | ||
| expect(markdownToHtml(editor.getHTML())).toMatch(/\|.*\|/); | ||
| } finally { | ||
| editor.destroy(); | ||
| } | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -240,12 +240,19 @@ | |
| } | ||
|
|
||
| table { | ||
| // `overflow-x` is inert on a `display: table` box, so a table wider than the | ||
| // post column was never scrollable, and the `overflow-hidden` utility below | ||
| // then clipped it, putting the right-hand columns permanently out of reach. | ||
| // `display: block` turns the table itself into the scroll container while its | ||
| // rows still lay out as a table internally; `width: 100%` keeps tables that | ||
| // already fit rendering full-width exactly as before. | ||
| display: block; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For tables narrower than the post column, Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and reverted in 5c73b5e. Measured at a 700px column: the narrow-table grid collapsed from 702px to 78px exactly as you describe, because the rows fell back to an anonymous shrink-to-fit table box while the element stayed at 100%. The underlying premise was also wrong: |
||
| word-break: normal !important; | ||
| overflow-x: auto; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- markdown styles ---'
sed -n '220,275p' apps/web/src/styles/_markdown.scss
printf '%s\n' '--- static body component ---'
sed -n '1,100p' apps/web/src/app/\(dynamicPages\)/entry/\[category\]/\[author\]/\[permlink\]/_components/entry-page-static-body.tsx
printf '%s\n' '--- renderPostBody references ---'
rg -n -C 4 'renderPostBody|markdown-view|sanitize|sanitiz' apps/web/src
printf '%s\n' '--- table and tabindex handling ---'
rg -n -C 3 'tabindex|tabIndex|<table|table.*overflow|overflow-x' apps/web/srcRepository: ecency/vision-web Length of output: 50374 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- markdown container styles ---'
sed -n '1,35p' apps/web/src/styles/_markdown.scss
sed -n '235,265p' apps/web/src/styles/_markdown.scss
printf '%s\n' '--- all relevant renderer call sites (limited) ---'
rg -l 'renderPostBody' apps/web/src | head -80
rg -n -C 2 'renderPostBody' \
'apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-static-body.tsx' \
'apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-pending-index-view.tsx' \
'apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-cross-post-body.tsx'
printf '%s\n' '--- dependency and workspace metadata ---'
rg -n -C 3 '"`@ecency/render-helper`"|render-helper' package.json package-lock.json yarn.lock pnpm-lock.yaml apps packages 2>/dev/null | head -250
printf '%s\n' '--- sanitizer/attribute configuration in tracked files ---'
rg -n -C 3 'ALLOWED_ATTR|allowedAttributes|allow.*tabindex|tabindex|sanitize-html|DOMPurify|sanitize' \
--glob '!**/*.spec.*' --glob '!**/node_modules/**' . | head -300Repository: ecency/vision-web Length of output: 50374 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- exact markdown rules ---'
sed -n '1,18p' apps/web/src/styles/_markdown.scss
sed -n '242,258p' apps/web/src/styles/_markdown.scss
printf '%s\n' '--- tracked render-helper and package files ---'
git ls-files | grep -E '(^|/)(render-helper|package(-lock)?\.json|yarn\.lock|pnpm-lock\.yaml)$' | head -200
printf '%s\n' '--- renderPostBody call-site markup ---'
python3 - <<'PY'
from pathlib import Path
needle = "renderPostBody"
for p in Path("apps/web/src").rglob("*"):
if not p.is_file() or p.suffix not in {".ts", ".tsx", ".js", ".jsx"}:
continue
try:
lines = p.read_text().splitlines()
except Exception:
continue
for i, line in enumerate(lines):
if needle in line:
print(f"{p}:{i+1}")
for n in range(max(0, i-3), min(len(lines), i+4)):
print(f" {n+1}: {lines[n]}")
print()
PY
printf '%s\n' '--- local tabindex and sanitizer references only ---'
rg -n -C 2 'tabindex|tabIndex|DOMPurify|sanitize-html|ALLOWED_ATTR|allowedAttributes' \
apps/web/src apps/web/package.json package.json --glob '!**/*.map' --glob '!**/generated/**' | head -250Repository: ecency/vision-web Length of output: 35129 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- render-helper package structure ---'
git ls-files packages/render-helper
printf '%s\n' '--- render-helper package metadata ---'
cat packages/render-helper/package.json
printf '%s\n' '--- renderer implementation symbols ---'
rg -n -C 4 'export.*renderPostBody|function renderPostBody|const renderPostBody|sanitize|tabindex|table' packages/render-helper --glob '!**/*.map' | head -300Repository: ecency/vision-web Length of output: 34827 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- allowed attributes ---'
cat -n packages/render-helper/src/consts/allowed-attributes.const.ts
printf '%s\n' '--- render pipeline ---'
rg -n -C 8 'renderPostBody|sanitizeHtml|markdownToHtml|traverse' packages/render-helper/src/index.ts packages/render-helper/src/helper.ts packages/render-helper/src/markdown-2-html.ts packages/render-helper/src/methods/markdown-to-html.method.ts packages/render-helper/src/methods/sanitize-html.method.ts
printf '%s\n' '--- existing table and sanitizer tests ---'
rg -n -C 6 'table|tabindex|attribute|whiteList' packages/render-helper/src/*.spec.ts packages/render-helper/src/methods/*.spec.tsRepository: ecency/vision-web Length of output: 50374 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
css = Path("apps/web/src/styles/_markdown.scss").read_text()
table = re.search(r'(?ms)^\s+table\s*\{(.*?)^\s+\}', css)
assert table, "table rule not found"
table_rule = table.group(1)
for declaration in ("display: block;", "overflow-x: auto;", "width: 100%;", "max-width: 100%;"):
assert declaration in table_rule, f"missing {declaration}"
attrs = Path("packages/render-helper/src/consts/allowed-attributes.const.ts").read_text()
table_attrs = re.search(r"'table':\s*\[(.*?)\]", attrs, re.S)
assert table_attrs, "table whitelist not found"
assert "tabindex" not in table_attrs.group(1).lower(), "table tabindex unexpectedly allowed"
renderer = Path("apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-static-body.tsx").read_text()
assert "dangerouslySetInnerHTML" in renderer
assert "renderPostBody(entry.body" in renderer
assert "tabIndex" not in renderer and "tabindex" not in renderer
pipeline = Path("packages/render-helper/src/markdown-2-html.method.ts").read_text()
assert "return sanitizeHtml(output)" in pipeline
print("table CSS creates an overflow scroll container")
print("rendered tables have no allowed tabindex attribute")
print("static entry body injects renderer HTML into the .markdown-view div")
print("renderer applies sanitizeHtml to final output")
PYRepository: ecency/vision-web Length of output: 719 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
css = Path("apps/web/src/styles/_markdown.scss").read_text()
table = re.search(r'(?ms)^\s+table\s*\{(.*?)^\s+\}', css)
assert table, "table rule not found"
table_rule = table.group(1)
for declaration in ("display: block;", "overflow-x: auto;", "width: 100%;", "max-width: 100%;"):
assert declaration in table_rule, f"missing {declaration}"
attrs = Path("packages/render-helper/src/consts/allowed-attributes.const.ts").read_text()
table_attrs = re.search(r"'table':\s*\[(.*?)\]", attrs, re.S)
assert table_attrs, "table whitelist not found"
assert "tabindex" not in table_attrs.group(1).lower(), "table tabindex unexpectedly allowed"
renderer = Path("apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-static-body.tsx").read_text()
assert "dangerouslySetInnerHTML" in renderer
assert "renderPostBody(entry.body" in renderer
assert "tabIndex" not in renderer and "tabindex" not in renderer
pipeline = Path("packages/render-helper/src/methods/markdown-to-html.method.ts").read_text()
assert "return sanitizeHtml(output)" in pipeline
print("table CSS creates an overflow scroll container")
print("rendered tables have no allowed tabindex attribute")
print("static entry body injects renderer HTML into the .markdown-view div")
print("renderer applies sanitizeHtml to final output")
PYRepository: ecency/vision-web Length of output: 367 Make the rendered table scroll region keyboard-accessible. The renderer emits tables without a focusable attribute, and its sanitizer removes 🤖 Prompt for AI AgentsSource: MCP tools |
||
| width: 100%; | ||
| max-width: 100%; | ||
|
|
||
| @apply border dark:border-gray-700 overflow-hidden table-auto border-collapse text-xs sm:text-sm md:text-base; | ||
| @apply border dark:border-gray-700 table-auto border-collapse text-xs sm:text-sm md:text-base; | ||
|
|
||
| tr { | ||
| @apply [&:last-child>td]:border-b-0 [&:nth-child(even)]:bg-light-200 dark:[&:nth-child(even)]:bg-dark-300; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an existing post contains a header table with block content in a cell, such as a list or blockquote, registering
tablesafter the custom table rule makes the plugin take precedence and converts the table to GFM even though GFM cells only support inline content. The conversion replaces the block structure with line-break-delimited Markdown, so merely editing the post silently changes or exposes that cell content on republish; the previoustablerule preserved such tables as HTML. Only route GFM-compatible tables through this plugin and retain the HTML fallback for cells with unsupported structure.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, and this one made me re-check the whole premise. The pre-existing
tablerule returningouterHTMLmeans develop already preserves tables losslessly, including cells with block content. Registeringtablesafter it trades that for GFM, which only holds inline content, so lists and blockquotes in cells degrade to<br>.Since the round-trip was never broken, that trade buys nothing. I have marked the PR draft and recommended closing it rather than adding a compatibility check, since the fix it claimed to make was not needed. Full write-up in the PR comment.