Powerpoint 2007 & ODPresentation Readers: Added support for loading embedded media #849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
January 28, 2025 11:55
…Image signature Fixed embed shape name to use just the filename
A presentation may have a label for the file (e.g. Media file 1), which differs from the actual file name (foobar.mp4)
Wilkolicious
commented
Jan 28, 2025
| @@ -791,7 +793,11 @@ protected function loadShapeDrawing(XMLReader $document, DOMElement $node, Abstr | |||
| { | |||
| // Core | |||
| $document->registerNamespace('asvg', 'http://schemas.microsoft.com/office/drawing/2016/SVG/main'); | |||
| if ($document->getElement('p:blipFill/a:blip/a:extLst/a:ext/asvg:svgBlip', $node)) { | |||
| $embedNode = $document->getElements("p:nvPicPr/p:nvPr//*[local-name()='media']", $node); | |||
| $embedNode = $embedNode ? $embedNode->item(0) : false; | |||
There was a problem hiding this comment.
The PHP Static Analysis (7.1) error doesn't look correct.
Ternary operator condition is always true.
$document->getElements() likely invokes and returns the result of DOMXPath::query, thus $embedNode may have a value of false:
If the expression is malformed or the contextNode is invalid, DOMXPath::query() returns false.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation
I work on behalf of an e-Learning authoring tool. We are adding powerpoint import as an option for client learning designers to quickly import proprietary information into their course designs.
We wanted to add support for importing the following data from a powerpoint:
I noticed that, although text and images were being loaded, there were no signs of embedded files. This PR aims to ameliorate that.
Changes
PowerPoint2007::loadShapeDrawingImageI tried not to deviate too far from existing code norms, and didn't over-optimise by refactoring common bits between loading shapes.
Limitations
Not sure where to start for Powerpoint 97 (PPT) formats. 🤔
Observations
The (untouched) parent classes of
MediaandFilelook very biased to images.Checklist: