Skip to content
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

Don't use IMG_BROKEN as png preview for all local / data url svg files on node #1315

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haakenlid
Copy link

@haakenlid haakenlid commented Jan 11, 2024

Fix issue #963

The problem is that the png preview of svgs are used by LibreOffice impress, while MS PowerPoint uses the svg directly.
However, Impress shows svg files fine, and don't need the png.

The same problem shows up in web preview of the pptx file if it's shared in Slack. I guess they maybe use LibreOffice to render the preview server side.

In the demo, the svg that is downloaded from Wikipedia show up fine, because PptxGenJS simply renames the svg to png and uses it as a "preview". However, the svg loaded from data url or from a local file will use the placeholder IMG_BROKEN png.

image

Here's the relevant part of the generated slide1.xml file for slide 1 created by the node demo.js Image example

Libreoffice will use the first embed "rId10", while powerpoint will display the inner one "rId11".

<a:blip r:embed="rId10">  // <- broken png image
  <a:alphaModFix amt="50000"/>
  <a:extLst>
    <a:ext uri="{96DAC541-7B7A-43D3-8B79-37D633B846F1}">
      <asvg:svgBlip xmlns:asvg="http://schemas.microsoft.com/office/drawing/2016/SVG/main" 
                             r:embed="rId11"/> // <- correct svg image
    </a:ext>
  </a:extLst>
</a:blip>

This PR will make local files and data urls work the same as http url svgs. The svg will be renamed to png and used as its own preview. Which seems to have caused no problems since #528 in 2019.

However. I don't really understand the reason behind the "png preview" and the <a:ext uri="{96DAC541-7B7A-43D3-8B79-37D633B846F1}"> tag at all. If you embed a svg using LibreOffice it will simply be included as <a:blip r:embed="rId10"> like any other image, and it seems to display fine everywhere.

Fix broken png preview
@haakenlid haakenlid changed the title Update gen-media.ts Don't use IMG_BROKEN as png preview for all local / data url svg files on node Jan 11, 2024
@haakenlid
Copy link
Author

haakenlid commented Jan 11, 2024

This PR should fix the bug in LibreOffice Impress, and still work fine with MS Powerpoint.

However in DropBox's web preview of uploaded powerpoint files, the "png" will not show up correctly. I suspect this system will not accept a svg file renamed to png.

image

But if I use LibreOffice and replace the embedded svg files with the proper svgs and save the file as .pptx, it looks good in DropBox also.

image
I've shared this fixed file here:
https://www.dropbox.com/scl/fi/m3i92si3b9rfwa9w06814/PptxGenJS_Image.fixed-with-libreoffice.pptx?rlkey=rb4rpnzkhzww3f5uxqsou4v1y&dl=0

Looking at the xml that LibreOffice creates, there's no .png preview file or <a:extLst> tag. All the xml tags used for svgs and other image formats are the same.

<p:pic>
  <p:nvPicPr>
    <p:cNvPr id="60" name="Image 6" descr="https://cdn.jsdelivr.net/gh/gitbrent/pptxgenjs@master/demos/common/images/lock-green.svg"/>
    <p:cNvPicPr/>
    <p:nvPr/>
  </p:nvPicPr>
  <p:blipFill>
    <a:blip r:embed="rId8"/>  // <- rId8 is lock-green.svg
    <a:stretch/>
  </p:blipFill>
  [ ... ]
</p:pic>

Which makes me wonder why PptxGenJS has this custom xml for svg files at all. Is this needed for very old versions of Microsoft Office?

Here's a link to the file after I fixed it with LibreOffice. The embedded svgs seems to display fine everywhere I have tried.
https://www.dropbox.com/scl/fi/m3i92si3b9rfwa9w06814/PptxGenJS_Image.fixed-with-libreoffice.pptx?rlkey=rb4rpnzkhzww3f5uxqsou4v1y&dl=0

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