Skip to content

Fix/docx import empty links - #2127

Open
samark231 wants to merge 2 commits into
mainfrom
fix/docx-import-empty-links
Open

Fix/docx import empty links#2127
samark231 wants to merge 2 commits into
mainfrom
fix/docx-import-empty-links

Conversation

@samark231

Copy link
Copy Markdown
Collaborator

Summary of Changes

Root-cause follow-up to the earlier slide-editor crash fix (the merged repairSlateChildren load-time repair, "C"). That fix contained the crash; this PR closes the source of the corruption and adds a safety net, so empty links stop being created and can never crash the editor again.

Background: Word .docx files with a Table of Contents / cross-references contain hidden heading bookmarks. On import, mammoth converts each to an empty <a id="_Toc…"></a>. These were saved raw, then round-tripped by the @yoopta/link (de)serializer into <a href="" …></a> — which becomes an invalid inline link node that crashes the Slate editor when it sits at the start of a heading. One real slide carried 44 of these.

Deliberately not included: a bulk DB migration ("D"). Existing affected slides already self-heal — on load via C, and on their next save via B — so no destructive backfill is needed.

Backend:

  • None — frontend only.

Frontend:

  • A — fix at import: convertDocToHtml now runs stripEmptyAnchors on the mammoth output, so Word bookmark anchors are removed before the HTML is ever saved (doc-to-html.ts).
  • B — fix on save: formatHTMLString (the choke point every serialize/save flows through) now strips content-less anchors, so a corrupt link can never be persisted — this also closes the paste vector, which bypasses the load-time repair (formatHtmlString.tsx).
  • Shared helper: added stripEmptyAnchors (one source of truth for A and B); the load path continues to use repairSlateChildren (C).
  • E — safety net: added EditorErrorBoundary around the document editor. If any future unsanitized corruption throws during render, it degrades to a graceful in-place message + a Sentry report instead of the whole app hitting the global "Something went wrong" page. It resets on slide-switch so one bad slide doesn't blank out the rest.

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

How Has This Been Tested?

  • Ran stripEmptyAnchors against the real production payload of the crashing slide (70 KB, 44 empty links): empty anchors 44 → 0, both genuine links preserved unchanged, all heading text intact ("From Script to Screen", "High Angle").
  • Verified the mammoth bookmark case (<h1><a id="_Toc…"></a>Heading</h1><h1>Heading</h1>) and that a real <a href="…">text</a> is left byte-identical.
  • tsc --noEmit (typecheck) passes clean.
  • EditorErrorBoundary mirrors the existing, in-use YooptaEditorWrapperSafe pattern (class boundary + Sentry.captureException).

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • I have updated the documentation accordingly

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying frontend-admin-dashboard with  Cloudflare Pages  Cloudflare Pages

Latest commit: da60a28
Status: ✅  Deploy successful!
Preview URL: https://f2b0c88f.frontend-admin-dashboard.pages.dev
Branch Preview URL: https://fix-docx-import-empty-links.frontend-admin-dashboard.pages.dev

View logs

@codacy-production

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 8 complexity

Metric Results
Complexity 8

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

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.

1 participant