dotnet / Open-XML-SDK

Open XML SDK by Microsoft
https://www.nuget.org/packages/DocumentFormat.OpenXml/
MIT License
4.01k stars 544 forks source link

Bug with Open XML SDK: PowerPoint could not open the file created by v2.18 with mc:AlternateContent element #1226

Open FullStackEngN opened 1 year ago

FullStackEngN commented 1 year ago

Describe the bug The customer generated PPTX documents by using OpenXML SDK, but PowerPoint would prompt Repair dialog while opening the document, and part of slide layout would be missed. This issue was not there when using OpenXML SDK 2.7.2 until upgraded OpenXML SDK to version 2.11~2.18.

Screenshots image

To Reproduce

Steps to reproduce the behavior:

  1. Use following code, to generate a new pptx file.
public static SlideLayoutPart CreateSlideLayout(PackEngineContext packEngineContext, SlideLayoutPart sourceLayoutPart, PresentationDocument targetDoc, SlideMasterPart targetMasterPart, List<ImageCacheItem> imageCacheItems, PPTIdCounter pptIdCounter)
{
    SlideLayoutPart targetLayoutPart = targetMasterPart.AddNewPart<SlideLayoutPart>();
    targetLayoutPart.FeedData(sourceLayoutPart);

    if (targetMasterPart.IsRepeatNameOfSlideLayout(targetLayoutPart))
    {
        targetLayoutPart.SetNameofSlideLayout();
    }

    targetLayoutPart.AddPart(targetMasterPart, targetDoc.PresentationPart.GetIdOfPart(targetMasterPart));

    bool needCloneDiagramParts = (sourceLayoutPart.DiagramColorsParts != null && sourceLayoutPart.DiagramColorsParts.Count() > 0) ||
                                  (sourceLayoutPart.DiagramDataParts != null && sourceLayoutPart.DiagramDataParts.Count() > 0);
    bool needCloneChartParts = sourceLayoutPart.HasChart();
    AddImageRelatedParts(sourceLayoutPart, sourceLayoutPart.SlideLayout.CommonSlideData, sourceLayoutPart.VmlDrawingParts,
                 targetLayoutPart, imageCacheItems, needCloneDiagramParts, needCloneChartParts);
    HandleOtherParts(packEngineContext, sourceLayoutPart, sourceLayoutPart.SlideLayout.CommonSlideData,
                 targetDoc, targetLayoutPart, targetLayoutPart.SlideLayout.CommonSlideData);

    SlideLayoutId newLayoutId = new SlideLayoutId();
    newLayoutId.Id = pptIdCounter.GetNextSlideMasterLayoutId();
    newLayoutId.RelationshipId = targetMasterPart.GetIdOfPart(targetLayoutPart);
    targetMasterPart.SlideMaster.SlideLayoutIdList.Append(newLayoutId);
    targetLayoutPart.SlideLayout.Save();

    return targetLayoutPart;
}

Observed behavior PowerPoint will pop up repair error message when opening the error.pptx.

Expected behavior No repair prompt and slide layout would not be broken.

Desktop (please complete the following information):

Additional context

  1. We compared the normal.pptx and error.pptx, then found the element order is different, if we manually change the element order, then PowerPoint can open the file successfully.

  2. The following code came from the directory &\ppt\slideMasters\slideMaster1.xml, XML node in PPTX generated by openxml2.7.2 is as follows, the element AlternateContent was behind sldLayoutIdLst, and the document can be opened normally. image

  3. In version 2.11~2.18 of OpenXML, the generated XML document changed to such that the "AlternateContent" and "p:sldLayoutIdLst" were reversed, causing this problem. image

mikeebowen commented 1 year ago

Hi @FullStackEngN, Thanks for submitting an issue. Could you please post the rest of your code to create a PPTX file. There are some undefined methods and classes with what you posted, so I'm unable to use your code to create a PPTX file and reproduce the issue.

If you need more info on creating a PPTX. There is a tutorial in our documentation: https://learn.microsoft.com/en-us/office/open-xml/how-to-create-a-presentation-document-by-providing-a-file-name

tomjebo commented 1 year ago

@FullStackEngN, to be more specific to what @mikeebowen requested, the function you included ( CreateSlideLayout()) does not show how the SlideMaster part is created which is where the transition element in question is added. Can you provide that function as well?

mahongbo86 commented 1 year ago

Hi @mikeebowen, @tomjebo , this question was submitted by me, because the code for creating pptx contains some business code, so it is not convenient to provide the complete code, but I can provide the steps when we process this document,

  1. we get a normal original document and create a PresentationDocument object for this original document, PresentationDocument originalDocument = PresentationDocument.Open(originalDocumentStream, false);

  2. The second step is to create a PresentationDocument object of the target document, MemoryStream targetDocStream = new MemoryStream(); using (PresentationDocument targetDocument = PresentationDocument.Create(targetDocStream, PresentationDocumentType.Presentation)) { }

  3. using the FeedData method, loads the information from the original document into the new var extendedFilePropertiesPart = targetDocument.AddNewPart<ExtendedFilePropertiesPart>(); extendedFilePropertiesPart.FeedData(originalDocument .ExtendedFilePropertiesPart); var corePropertiesPart = targetDocument.AddNewPart<CoreFilePropertiesPart>(); corePropertiesPart.FeedData(sourceDocument.CoreFilePropertiesPart); var custPropPart = targetDocument.AddCustomFilePropertiesPart(); custPropPart.FeedData(sourceDocument.CustomFilePropertiesPart);

One of the steps is to use the method CreateSlideLayout mentioned in the ticket

tomjebo commented 1 year ago

@mahongbo86, so you are copying an existing presentation to a new presentation using FeedData, is this correct? Then is my assumption that you are doing FeedData for the SlideMaster part as well? If so, that helps and I can try to reproduce this method of creating a new document. It would help to have a minimal repro sample to work with. This is something that typically is a requirement for these kind of issues.

tomjebo commented 1 year ago

So FeedData uses Stream.CopyTo which is not the SDK but System.IO. I'm not sure yet if the problem happens there but looks like that's the likely place unless the customer's code is adding a transition element after the fact. And that's all the more reason we need the actual code that creates (even if it's via FeedData) the SlideMaster part not just the SlideLayout part. Please provide that code for us.

mahongbo86 commented 1 year ago

@tomjebo SlideMaster We also use FeedData, I can provide this part of the code.

public static SlideMasterPart CreateSlideMaster(PackEngineContext packEngineContext, SlideMasterPart sourceMasterPart, PresentationDocument targetDoc, bool isDefaultMaster, List<ImageCacheItem> imageCacheItems, PPTIdCounter pptIdCounter)
{
    SlideMasterPart targetMasterPart = targetDoc.PresentationPart.AddNewPart<SlideMasterPart>();
    targetMasterPart.FeedData(sourceMasterPart);
    targetMasterPart.SlideMaster.SlideLayoutIdList = new SlideLayoutIdList();

    bool needCloneDiagramParts = (sourceMasterPart.DiagramColorsParts != null && sourceMasterPart.DiagramColorsParts.Count() > 0) ||
                                   (sourceMasterPart.DiagramDataParts != null && sourceMasterPart.DiagramDataParts.Count() > 0);
    bool needCloneChartParts = sourceMasterPart.HasChart();
    AddImageRelatedParts(sourceMasterPart, sourceMasterPart.SlideMaster.CommonSlideData, sourceMasterPart.VmlDrawingParts,
                targetMasterPart, imageCacheItems, needCloneDiagramParts, needCloneChartParts);
    HandleOtherParts(packEngineContext, sourceMasterPart, sourceMasterPart.SlideMaster.CommonSlideData,
                targetDoc, targetMasterPart, targetMasterPart.SlideMaster.CommonSlideData);

    ThemePart targetThemePart = AddThemePart(targetMasterPart, sourceMasterPart.ThemePart, imageCacheItems);
    targetThemePart.Theme.Save();

    if (targetDoc.PresentationPart.IsRepeatNameOfSlideMaster(targetMasterPart))
    {
        targetMasterPart.SetNameOfSlideMaster();
    }

    if (isDefaultMaster)
    {
        if (targetDoc.PresentationPart.ThemePart != null)
        {
            targetDoc.PresentationPart.DeletePart(targetDoc.PresentationPart.ThemePart);
        }
        targetDoc.PresentationPart.AddPart<ThemePart>(targetThemePart);
    }

    SlideMasterIdList slideMasterIdList = targetDoc.PresentationPart.Presentation.SlideMasterIdList;
    SlideMasterId newMasterId = new SlideMasterId();
    newMasterId.RelationshipId = targetDoc.PresentationPart.GetIdOfPart(targetMasterPart);
    newMasterId.Id = pptIdCounter.GetNextSlideMasterLayoutId();
    slideMasterIdList.Append(newMasterId);
    targetMasterPart.SlideMaster.Save();

    return targetMasterPart;
}
tomjebo commented 1 year ago

Thank you.

tomjebo commented 1 year ago

@mahongbo86, can you also provide the source and target slidemaster.xml parts? And it would help if you can step through the problem code to see if the FeedData is making this change or not.

mahongbo86 commented 1 year ago

@tomjebo source.pptx target.pptx These two pptx files can be provided, and the target has the problem of this error prompt.

tomjebo commented 1 year ago

I created test code that simply uses FeedData to open a PowerPoint created presentation and add a new SlideMaster part copied from the original SlideMaster. The original SlideMasterPart has ACB in it copied from yours. The new SlideMasterPart is identical to the original including order of the ACB in relation to the other elements. So I don't think FeedData is the problem.

tomjebo commented 1 year ago

Since you aren't able to provide a simple repro, my suggestion would be to step through the customer's code, watching the part in question (you can view the XML in the debugger inspector) and report to us after which operation or line the part changes. This is what I would do if I had a reproducible example.

mahongbo86 commented 1 year ago

I will provide a fully reproducible demo of the problem, but it will take a while, I will update it here when I am done

tomjebo commented 1 year ago

Ok, actually, I found out that your line here:

targetMasterPart.SlideMaster.SlideLayoutIdList = new SlideLayoutIdList();

after the feeddata call is the one that causes the order change. I don't know why yet but that's where it happens. You don't need to do the simple repro now if you don't have time. I will debug this and find out if we changed something.

tomjebo commented 1 year ago

@mahongbo86 @FullStackEngN

There is a problem with replacing the element block contained in SlideMaster.SlideLayoutIdList when any of the sequence elements in the p\:sldMaster block are enclosed in an AlternateContentBlock. I'm not sure if I can fix this quickly but will investigate options to fix it. In the meantime, I would recommend you use the following as a work around.

Where you do this:

    SlideMasterPart targetMasterPart = targetDoc.PresentationPart.AddNewPart<SlideMasterPart>();
    targetMasterPart.FeedData(sourceMasterPart);
    targetMasterPart.SlideMaster.SlideLayoutIdList = new SlideLayoutIdList();

I recommend you instead keep the existing SlideLayoutIdList object and just replace the contents or children like this:

   SlideMasterPart targetMasterPart = targetDoc.PresentationPart.AddNewPart<SlideMasterPart>();

   targetMasterPart.FeedData(newPreDoc.PresentationPart.GetPartsOfType<SlideMasterPart>().FirstOrDefault().GetStream());
   targetMasterPart.SlideMaster.SlideLayoutIdList.RemoveAllChildren();

   // example adding one layout id

   SlideLayoutId newLayoutId = new SlideLayoutId();
   newLayoutId.Id = 2147483653; 
   newLayoutId.RelationshipId = "rId5";

   // ... add more layout id's ...

   targetMasterPart.SlideMaster.SlideLayoutIdList.Append(newLayoutId);

Effectively, I think you could just replace:

    targetMasterPart.SlideMaster.SlideLayoutIdList = new SlideLayoutIdList();

with:

   targetMasterPart.SlideMaster.SlideLayoutIdList.RemoveAllChildren();

I tested the approach and it does work. Please try this and let me know if you have questions.

mahongbo86 commented 1 year ago

hi @tomjebo @FullStackEngN I tried the solution you provided, but still have problems. Below is our implementation code modified according to your suggestions.

SlideMasterPart targetMasterPart = targetDoc.PresentationPart.AddNewPart<SlideMasterPart>(); var stream = packEngineContext.DocContext.Document.PresentationPart.GetPartsOfType<SlideMasterPart>().FirstOrDefault().GetStream(); targetMasterPart.FeedData(stream); targetMasterPart.SlideMaster.SlideLayoutIdList.RemoveAllChildren();

This is a problem with the documentation after generation

image

image

We currently use this scheme to solve this problem image

tomjebo commented 1 year ago

@mahongbo86 The original source and target sample files you provided don't have a pic or blipFill in them and are working for me which leads me to think I don't have the files for your current problem. If that's true, can you please provide the before and after files that you're using for this last problem?

mahongbo86 commented 1 year ago

@tomjebo OK, Today I will provide this original document and the target document.

mahongbo86 commented 1 year ago

original.pptx target-has error.pptx

@tomjebo
The two documents origin are the original documents, The document generated by target using the scheme you provided still has a pop-up prompt

image

image

tomjebo commented 1 year ago

@mahongbo86 thanks for providing the two files, I'll take a look.