internetarchive / internetarchive/openlibrary

WYSIWYG editor rewrites raw HTML in /type/page bodies on save (79% of collections pages affected)

Open
#13,442 1 comment 0 reactions 1 assignee Claimed by @lokesh View on GitHub
Affects: Data Lead: @mekarpeles Module: JavaScript Needs: Review Assignee Needs: Staff Decision Priority: 2 Theme: Editing Type: Bug
Dominant language
Python
Stars
6.7k
Forks
2k
Avg merge
2d 19h
Merged PRs (30d)
138

Description

## Summary

Opening a `/type/page` wiki page in the WYSIWYG editor and saving it rewrites the entire body through a markdown round-trip. Raw HTML — which 70% of our collections pages depend on — does not survive that round-trip intact. **71 of 89 live collections pages (79%) are altered by a single open-and-save**, and a tail of them lose real content: link targets, HTML entities, and inline markup.

This is not hypothetical. It happened to https://openlibrary.org/collections/nebula-awards today (r55 → r57), which lost every section heading, every anchor link, and its whole in-page nav. I've restored it as r58 with the r55 body.

## What happened on the Nebula page

| revision | body length | state |
|---|---|---|
| 55 | 3292 | good |
| 56 | 2989 | inline HTML rewritten to markdown, entities decoded |
| 57 | 2123 | all `

` section headings gone, all links reduced to plain text |
| 58 | 3292 | restored (r55 body, posted directly to `?m=edit` to bypass the editor) |

The `{{QueryCarousel(...)}}` macros survived throughout, which is why the page still showed books — just with no headings, no anchor nav, and no links.

## Root cause

`openlibrary/templates/type/page/edit.html:30` mounts `` over the body textarea:

```html
$page.body

```

The editor's `onUpdate` (`OLMarkdownEditor.js:459`) reserializes the **entire** document on every change and overwrites the textarea:

```js
let markdownOutput = editor.storage.markdown.getMarkdown();
this.targetElement.value = markdownOutput;
```

So a one-character edit anywhere rewrites the whole body. The pipeline is:

```
markdown source
→ markdown-it render (html: true)
→ HTML string
→ ProseMirror DOMParser against the editor schema ← anything the schema can't model is dropped
→ tiptap-markdown serializer
→ markdown source'
```

The schema is StarterKit + `Image` + `HtmlBlock` (`editor-core.js`). `HtmlBlock` catches markdown-it `html_block` tokens and does preserve block-level HTML verbatim — that part works, and it's why tables and `

` blocks survive a normal load-and-save. What it does not cover is **inline** HTML (`html_inline` tokens), which is reparsed against the schema and reduced to whatever the schema happens to support.

Two notes for whoever picks this up:

- `HtmlBlock` is added unconditionally in `createEditor()`. The `enable-html-block` attribute only controls whether the toolbar button renders, not whether HTML blocks round-trip.
- The display renderer is a *different* markdown implementation — `OLMarkdown`, a vendored Python-Markdown 1.6b (`openlibrary/core/olmarkdown.py`) — while the editor parses with markdown-it/CommonMark. The two dialects disagree, and we've already shipped one fix for that disagreement (#13074, hard breaks). Every additional feature we lean on widens that gap.

## Measured blast radius

I ran every live page body through a faithful reproduction of the editor pipeline (same versions, same extension config, jsdom; results confirmed against the real editor in Chrome, which produced byte-identical output).

**Collections pages (89 with a body):**

| | |
|---|---|
| altered by one round-trip | **71 (79%)** |
| lose a link target (real breakage) | 3 pages, 16 of 1422 links |
| lose HTML entities | 12 pages |
| lose an anchor `id` | 0 |
| lose a table or a carousel macro | 0 |
| total body bytes | 501,246 → 492,367 (−1.8%) |

**Non-collection wiki pages (54 sampled from `/about`, `/help`, `/dev`, `/tour`, `/developers`):**

| | |
|---|---|
| contain raw HTML | 33 |
| lose a link target | 9 |
| lose HTML entities | 9 |

`/dev/docs/api/authors` loses both of its two links — 100% loss on that page.

There are **495 `/type/page` docs** in production (191 `/collections` including translated variants, 36 `/about`, 36 `/dev`, 30 `/help`, 7 `/community`, 7 `/tour`). Raw HTML across collections bodies: `td`×5332, `a`×2289, `tr`×778, `h2`×700, `h3`×430, `dd`×100, `hr`×98, `dt`×90, `dl`×52, `img`×48.

**Good news, and it corrects my first read of this:** the round-trip **converges**. All 89 pages reach a fixed point, nearly all by pass 2–3 — pass 1 costs 1.8%, and passes 2–5 cost a further 0.01% combined. This is one lossy normalization, not runaway decay. Any fix should be judged against that: we are protecting a one-time ~2% haircut plus a small tail of genuinely broken links, not a page that dissolves.

## What I could not reproduce

The r55 → r56 transition reproduces exactly (my harness turns r55 into 2934 chars; r56 is 2989, the difference being the actual edit that was made). **The r56 → r57 step does not.** Loading r56 and reserializing yields 2920 chars with all HTML blocks intact; the real r57 is 2123 with the blocks emptied and link marks stripped. I tested and ruled out the source-view toggle path (`_toggleSource` → `setContent`, lossless) and repeated round-trips (converged).

So there is a second, more destructive failure mode reachable through some in-editor interaction — most likely a paste, a select-all-and-retype, or editing inside the `html-block` textareas — that I did not isolate. Worth reproducing before signing off on any fix, because a fix aimed only at the round-trip may not close it.

## Options

**A. Improve HTML fidelity inside Tiptap.** Add the missing schema nodes (`@tiptap/extension-table`, definition lists) and a custom node/mark that preserves arbitrary *inline* HTML verbatim the way `HtmlBlock` does for block HTML.

Against it: ProseMirror drops what its schema can't model, by design. Chasing verbatim HTML fidelity means modelling all of HTML, which is building an HTML editor rather than a markdown one, and it's permanent maintenance. It also doesn't help the second failure mode above.

**B. Migrate to Tiptap's official markdown extension.** We should do this regardless — `tiptap-markdown@0.9.0` is **abandoned**. Its author's notice: Tiptap shipped a markdown extension in 3.7.0, prefer it, "I don't plan to address current issues / PR." Last push 2025-10-22. We're already on `@tiptap/core@3.26.0`, so the official extension is available to us.

Against it as a *fix for this bug*: it's the same architecture with the same constraint — anything without a CommonMark equivalent is dropped — it's flagged in Tiptap's own docs as an early release "subject to change or may have edge cases that may not be supported yet", and it has its own open round-trip data-loss reports (ueberdosis/tiptap#7147, #7731). Treat it as dependency hygiene, not as the fix.

**C. Make `/type/page` a plain markdown/source editor.** Don't mount the WYSIWYG on this one template; edit the text directly. Losslessness is structural — no parse, no serialize, no round-trip.

Note that `/type/page` is already the odd one out: it's the only one of the nine `ol-markdown-editor` mounts that passes `enable-html-block enable-code`. The other eight (work/edition descriptions, author bios, list and user descriptions, tags) are short prose fields where markdown is the right model and the WYSIWYG is a genuine win. This option leaves all of those alone.

Against it: gives up WYSIWYG editing for wiki pages, which is what #12182 set out to provide.

**D. Open `/type/page` in source mode, WYSIWYG opt-in.** Cheaper politically than C, but the toggle back into WYSIWYG re-enters the same pipeline, so it only narrows the window rather than closing it. Would need a true source-only mode that never mounts the editor.

**E. Only write back what actually changed.** Serialize just the blocks the user touched and leave the rest byte-identical. Correct in principle and it fixes the real complaint — untouched content should never change. In practice it's hard and fragile against ProseMirror's document model, and it's a large piece of work.

**F. Server-side guard.** Reject or interstitially confirm a save that drops HTML tags, links, or a large fraction of the body. Doesn't fix the editor, but it's the only option that also catches the unreproduced failure mode, plus future regressions, plus bad pastes.

**G. Change the storage format to HTML.** Would require migrating 495 pages and reworking `OLMarkdown` rendering. Disproportionate, and markdown is the friendlier authoring format.

## Recommendation

**C + F, in that order, and treat B as separate hygiene.**

1. **Now — stop mounting the WYSIWYG on `/type/page` (option C).** One-line template change, zero data loss, ships today. Wiki-page bodies are the one place in the codebase where raw HTML is a first-class authoring idiom — 70% of collections pages, 5332 ``s, hand-written anchor navigation — and a markdown WYSIWYG is the wrong tool for content that is substantially not markdown. Keep the WYSIWYG everywhere else; those eight fields are prose and are not affected.

2. **Next — add the server-side guard (option F).** A save that removes HTML tags or link targets should require explicit confirmation. This is the only measure that covers the r56 → r57 mode I couldn't reproduce, and it keeps protecting us whatever we do with the editor later.

3. **Separately — migrate off `tiptap-markdown` (option B).** It's an unmaintained dependency in the path of nine edit forms. That's worth doing on its own schedule, and it is not a fix for this issue.

4. **Only then, if WYSIWYG on wiki pages is still wanted**, revisit A or E — with a corpus round-trip test in CI as the gate. The harness I used for the numbers above is a good starting point: pull every `/type/page` body, round-trip it, and fail on any lost link target, anchor id, or HTML tag. Nothing should ship WYSIWYG for `/type/page` again until that test is green.

## Reproducing the measurements

The corpus test is worth having in the repo. The shape of it:

```js
// jsdom globals first, then mirror openlibrary/components/lit/editor-core.js exactly:
const editor = new Editor({
element,
extensions: [
StarterKit.configure({
heading: { levels: [1, 2, 3, 4] },
codeBlock: undefined, code: undefined,
link: { openOnClick: false, autolink: true },
strike: false, hardBreak: false,
}),
OLHardBreak,
Markdown.configure({ breaks: true, linkify: true }),
Image.configure({ inline: true, allowBase64: false }),
HtmlBlock,
],
content: body.replace(/\r\n/g, '\n'), // the browser normalises CRLF in the textarea
});
const out = editor.storage.markdown.getMarkdown();
// assert: no link href, anchor id, or HTML tag present in `content` is missing from `out`
```

Fetch bodies from `https://openlibrary.org/query.json?type=/type/page&limit=1000` and `.json`.

## Restoring a mangled page

For anyone who hits this before it's fixed — the WYSIWYG will re-mangle the page if you use the edit form, so post the old revision back directly:

```bash
curl "https://openlibrary.org/.json?v=" | jq -r .body.value > body.txt

curl -X POST "https://openlibrary.org/?m=edit" -b "session=" \
--data-urlencode "type.key=/type/page" \
--data-urlencode "title=" \
--data-urlencode "body@body.txt" \
--data-urlencode "_comment=Restore revision <n>" \
--data-urlencode "_save=Save"
```

There is also a built-in `?m=revert&v=<n>` POST mode (`openlibrary/plugins/upstream/code.py:262`), but it requires super-librarian or higher and has no UI.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.