Skip to content

Add skipFormatContainerFallbackCheck to keep persisted Content Model path valid#3358

Open
JiuqingSong wants to merge 1 commit into
masterfrom
fix-format-container-fallback-path
Open

Add skipFormatContainerFallbackCheck to keep persisted Content Model path valid#3358
JiuqingSong wants to merge 1 commit into
masterfrom
fix-format-container-fallback-path

Conversation

@JiuqingSong
Copy link
Copy Markdown
Collaborator

Summary

When formatInsertPointWithContentModel persists the Content Model group path during DOM-to-Model conversion (so the formatting callback can later reference it), a FormatContainer that gets created for a container element (e.g. a <div> with margin/padding) can later fall back to a plain paragraph when it only wraps a single paragraph. That fallback removes the FormatContainer from the model, leaving the persisted path pointing at a node that no longer exists — an invalid path.

This adds a skipFormatContainerFallbackCheck option that keeps the FormatContainer in that scenario, so the persisted path stays valid and the formatting callback works correctly.

Original bug: https://outlookweb.visualstudio.com/Outlook%20Web/_workitems/edit/426559

Changes

  • formatContainerProcessor: skip the paragraph-fallback when context.skipFormatContainerFallbackCheck is set.
  • DomToModelOption / DomToModelSettings: add the new skipFormatContainerFallbackCheck flag.
  • createContentModel: propagate the option onto the DOM-to-Model context (set only when truthy, matching the existing if (selection) pattern so the context isn't polluted with an undefined key).
  • formatInsertPointWithContentModel: pass skipFormatContainerFallbackCheck: true, with a comment explaining why.

Tests

  • New formatContainerProcessor tests for the skip behavior (div / div-with-id kept as FormatContainer; blockquote unaffected).
  • createContentModel wiring tests (option propagated when set, absent otherwise).
  • Updated formatInsertPointWithContentModel assertions to expect the new option.

All affected tests pass via yarn test:fast, and yarn eslint is clean.

🤖 Generated with Claude Code

…path valid

When formatInsertPointWithContentModel persists the Content Model group path during DOM to Model conversion, a FormatContainer that falls back to a plain paragraph would be removed from the model, leaving the persisted path invalid. Add a skipFormatContainerFallbackCheck option that keeps the FormatContainer in that scenario so the formatting callback can still rely on the path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://microsoft.github.io/roosterjs/pr-preview/pr-3358/

Built to branch gh-pages at 2026-05-29 19:59 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a DOM-to-Content-Model conversion option to preserve FormatContainer nodes when formatInsertPointWithContentModel needs a stable persisted group path during formatting.

Changes:

  • Adds skipFormatContainerFallbackCheck to create-model options/settings.
  • Propagates the option through createContentModel and applies it in formatContainerProcessor.
  • Enables the option for formatInsertPointWithContentModel and updates/adds related tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/roosterjs-content-model-types/lib/context/DomToModelSettings.ts Adds runtime setting for skipping format-container fallback.
packages/roosterjs-content-model-types/lib/context/DomToModelOption.ts Exposes the new create-model option.
packages/roosterjs-content-model-dom/lib/domToModel/processors/formatContainerProcessor.ts Preserves FormatContainer when the new context flag is set.
packages/roosterjs-content-model-dom/lib/domToModel/context/createDomToModelContext.ts Adds explicit return type to context factory helper.
packages/roosterjs-content-model-dom/test/domToModel/processors/formatContainerProcessorTest.ts Covers skip-fallback behavior for div containers and blockquote.
packages/roosterjs-content-model-core/lib/coreApi/createContentModel/createContentModel.ts Propagates the option onto the DOM-to-model context.
packages/roosterjs-content-model-core/test/coreApi/createContentModel/createContentModelTest.ts Tests option propagation and absence when unset.
packages/roosterjs-content-model-api/lib/publicApi/utils/formatInsertPointWithContentModel.ts Enables the option for insert-point formatting to keep persisted paths valid.
packages/roosterjs-content-model-api/test/publicApi/utils/formatInsertPointWithContentModelTest.ts Updates assertions for the new DOM-to-model override option.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants