-
Notifications
You must be signed in to change notification settings - Fork 179
Fix 230556 #3223
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
Fix 230556 #3223
Conversation
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.
Pull request overview
This PR adds support for legacy HTML table attributes (border, cellpadding, cellspacing) that were previously ignored because they are deprecated in HTML5. These attributes are still used in some email clients and legacy scenarios, so preserving them improves content fidelity when replying to emails with tables.
Key changes:
- Introduces a new
LegacyTableBorderFormattype to store legacy table border attributes - Adds a format handler to parse and apply these legacy attributes
- Updates table normalization logic to skip border collapse for legacy-style tables
- Removes test data that contained these attributes to avoid conflicts with the new preservation behavior
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/roosterjs-content-model-types/lib/contentModel/format/formatParts/LegacyTableBorderFormat.ts |
Defines the new type for legacy table border format with properties for border, cellspacing, and cellpadding |
packages/roosterjs-content-model-types/lib/index.ts |
Exports the new LegacyTableBorderFormat type |
packages/roosterjs-content-model-types/lib/contentModel/format/FormatHandlerTypeMap.ts |
Registers the legacy table border format in the format handler type map |
packages/roosterjs-content-model-types/lib/contentModel/format/ContentModelTableFormat.ts |
Extends ContentModelTableFormat to include LegacyTableBorderFormat |
packages/roosterjs-content-model-dom/lib/formatHandlers/table/legacyTableBorderFormatHandler.ts |
Implements the format handler for parsing and applying legacy table attributes |
packages/roosterjs-content-model-dom/lib/formatHandlers/defaultFormatHandlers.ts |
Registers the new legacy table border format handler in the default handlers |
packages/roosterjs-content-model-dom/lib/formatHandlers/table/tableSpacingFormatHandler.ts |
Removes logic that was treating cellPadding as a trigger for border collapse |
packages/roosterjs-content-model-dom/lib/modelApi/editing/normalizeTable.ts |
Updates normalization to skip border collapse when legacy table attributes are present |
packages/roosterjs-content-model-dom/test/formatHandlers/table/legacyTableBorderFormatHandlerTest.ts |
Adds comprehensive tests for the new format handler |
packages/roosterjs-content-model-dom/test/formatHandlers/table/tableSpacingFormatHandlerTest.ts |
Removes test for cellPadding triggering border collapse |
packages/roosterjs-content-model-plugins/test/paste/word/processPastedContentFromWacTest.ts |
Removes border attribute from test HTML |
packages/roosterjs-content-model-plugins/test/paste/e2e/htmlTemplates/htmlFromExcelNonNative.ts |
Removes legacy attributes from Excel template |
packages/roosterjs-content-model-plugins/test/paste/e2e/cmPasteFromWacTest.ts |
Removes border attribute from Word Online test HTML |
.vscode/settings.json |
Adds 'cellspacing' to the spell check dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bug 230556: Re: Monarch Feedback 🐞 - Mail - Replying to mail breaks table formatting. The borders/lines between the table cells are missing in the reply.
We didn't support the legacy table attributes (border, cellpadding, cellspacing) because they are deprecated. But in some scenarios they are still used. So add support to them to help improve content fidelity.