Skip to content

Fix ChangeDocumentType not removing VBA parts when converting to non-macro type#2039

Open
iamcymentho wants to merge 1 commit intodotnet:mainfrom
iamcymentho:iamcymentho/fix-change-document-type-vba-cleanup
Open

Fix ChangeDocumentType not removing VBA parts when converting to non-macro type#2039
iamcymentho wants to merge 1 commit intodotnet:mainfrom
iamcymentho:iamcymentho/fix-change-document-type-vba-cleanup

Conversation

@iamcymentho
Copy link
Copy Markdown

Summary

Fixes #618

When changing a macro-enabled document (.docm) to a standard document (.docx) via ChangeDocumentType(), VBA-related parts (VbaProjectPart, VbaDataPart, etc.) were carried over to the new document type unchanged. This left orphaned vbaProject content type entries in [Content_Types].xml, producing a file that:

  • Could not be opened correctly by some tools
  • Silently retained macro capabilities in a supposedly non-macro document

Changes

  • TypedPackageFeatureCollection.cs — After ChangeDocumentTypeInternal completes, check if the new content type is non-macro-enabled. If so, remove parts with VBA relationship types (vbaProject, keyMapCustomizations) from the new main part.
  • DocxTests01.cs — Added test that creates a .docm with a VbaProjectPart, converts it to .docx, and verifies:
    1. VbaProjectPart is null after conversion
    2. The vbaProject content type is absent from the package

Test results

Test Project Total Passed Failed
DocumentFormat.OpenXml.Tests 2316 2313 0
DocumentFormat.OpenXml.Framework.Tests 192 192 0
DocumentFormat.OpenXml.Packaging.Tests 190 190 0

Test plan

  • New test ChangeDocumentType_MacroEnabledToDocument_RemovesVbaParts passes
  • All 3 existing ChangeDocumentType tests still pass
  • Full test suite (2698 tests) passes with 0 failures

@iamcymentho
Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 20, 2026

Test Results

    68 files  ± 0      68 suites  ±0   1h 42m 33s ⏱️ + 5m 8s
 2 069 tests + 1   2 066 ✅ + 1   3 💤 ±0  0 ❌ ±0 
38 050 runs   - 16  38 008 ✅  - 16  42 💤 ±0  0 ❌ ±0 

Results for commit b50732c. ± Comparison against base commit 3ee250c.

♻️ This comment has been updated with latest results.

@mikeebowen mikeebowen requested a review from twsouthwick March 20, 2026 18:25
Comment thread src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Outdated
Comment thread src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Outdated
Comment thread src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Outdated
Comment thread src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Outdated
Comment thread src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Outdated
@iamcymentho iamcymentho force-pushed the iamcymentho/fix-change-document-type-vba-cleanup branch from 9f5dfb7 to b50732c Compare March 20, 2026 22:09
@iamcymentho
Copy link
Copy Markdown
Author

Thanks for the feedback @twsouthwick @mikeebowen! I've reworked the implementation:

  • Moved VBA cleanup out of the shared TypedPackageFeatureCollection into WordprocessingDocumentFeatures
  • Added a virtual OnDocumentTypeChanged hook in the base class so each document type can handle its own cleanup
  • Replaced the HashSet + LINQ approach with direct DeletePartsOfType<VbaProjectPart>() and DeletePartsOfType<CustomizationPart>() calls , no LINQ, no extra using
  • Uses enum pattern matching (is MacroEnabledDocument or MacroEnabledTemplate) instead of string checks

If Excel/PowerPoint need the same treatment, their feature classes can override OnDocumentTypeChanged independently.

Copy link
Copy Markdown

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

Fixes Word ChangeDocumentType() conversion from macro-enabled (.docm) to non-macro (.docx/.dotx) by adding a post-conversion cleanup hook and using it to remove VBA-related parts after the main part content type is switched (addresses #618).

Changes:

  • Add a generic OnDocumentTypeChanged(...) hook to TypedPackageFeatureCollection and invoke it after ChangeDocumentTypeInternal.
  • Override the hook for WordprocessingDocument to delete VbaProjectPart and CustomizationPart when converting to non-macro types.
  • Add a test that creates a .docm, converts to .docx, and asserts VBA parts are removed.

Reviewed changes

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

File Description
test/DocumentFormat.OpenXml.Tests/DocxTests01.cs Adds coverage for .docm.docx conversion and VBA removal assertions.
src/DocumentFormat.OpenXml/Packaging/WordprocessingDocument.cs Implements Word-specific cleanup after document type changes to non-macro types.
src/DocumentFormat.OpenXml/Packaging/TypedPackageFeatureCollection.cs Introduces and calls a post-document-type-change hook for typed package feature collections.

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

Comment thread test/DocumentFormat.OpenXml.Tests/DocxTests01.cs Outdated
Comment thread src/DocumentFormat.OpenXml/Packaging/WordprocessingDocument.cs
@iamcymentho iamcymentho force-pushed the iamcymentho/fix-change-document-type-vba-cleanup branch from b50732c to b460fae Compare March 21, 2026 16:22
…macro type

When changing a macro-enabled document (.docm) to a standard document (.docx)
via ChangeDocumentType(), VBA-related parts (VbaProjectPart, CustomizationPart)
were carried over to the new document type, leaving orphaned content type entries.

Add a virtual OnDocumentTypeChanged hook in TypedPackageFeatureCollection and
override it in WordprocessingDocumentFeatures to recursively remove VBA parts
when the new type is not macro-enabled.

Fixes dotnet#618
@iamcymentho
Copy link
Copy Markdown
Author

@twsouthwick @mikeebowen Friendly follow-up, this is a reminder about latest push which addresses all review feedback. The only open question is whether to extend the VBA cleanup to Excel/PowerPoint in this PR or a separate one. Happy to proceed either way once you've had a chance to align.

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.

ChangeDocumentType does not fully remove vbaProject reference

4 participants