I am trying to Merge some slides from different Powerpoint files to a new target file which is a mix of slides from files with the same base layout. I had to work around some issues e.g. avoiding null pointers and such and some of these changes have made it into the current SNAPSHOT. After these changes I got the of import problems down. Of 157 slides I some 10 have a problem e.g. when formatting options of recent powerpoint versions have been used that seem not to be supported. I usually then simply take a screen shot of these and replace the result with the bitmap. This all happened on a laptop with the most recent Mac OS. Now I wanted to finish on this project while I am on the road and moved the project to a different laptop running Mac OS X 10.9.5. Surprise - more errors show up again- there seem to be problems with fonts and images that where not happening (any more?) on the other OS. I'll have to investigate whethere the issues is with not picking up the Eclipse patched POI at all or with POI itself. For the time being I am opening up this bug on the assumption that the patched POI is being used. Nevertheless to get things working the patches seem to be necessary. = Patches= diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java index 09fb30f..6180175 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java @@ -388,7 +388,10 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack * @see org.apache.poi.openxml4j.opc.RelationshipSource#getRelationship(java.lang.String) */ public PackageRelationship getRelationship(String id) { - return this._relationships.getRelationshipByID(id); + if (id==null) + return null; + PackageRelationship result = this._relationships.getRelationshipByID(id); + return result; } /** @@ -457,7 +460,16 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack * @see org.apache.poi.openxml4j.opc.RelationshipSource#isRelationshipExists(org.apache.poi.openxml4j.opc.PackageRelationship) */ public boolean isRelationshipExists(PackageRelationship rel) { - return _relationships.getRelationshipByID(rel.getId()) != null; + // should we throw an IllegalArgumentException here? + if (rel==null) + return false; + String id=rel.getId(); + // how do we avoid the id to be null in the first place? + if (id==null) { + return false; + } + PackageRelationship rs = _relationships.getRelationshipByID(id); + return rs!= null; } /** diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationshipCollection.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationshipCollection.java index 953af91..2ed15d3 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationshipCollection.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationshipCollection.java @@ -197,7 +197,12 @@ public final class PackageRelationshipCollection implements * The relationship to add. */ public void addRelationship(PackageRelationship relPart) { - relationshipsByID.put(relPart.getId(), relPart); + String id=relPart.getId(); + if (id==null) { + // should we throw an IllegalArgumentException here? + return; + } + relationshipsByID.put(id, relPart); relationshipsByType.put(relPart.getRelationshipType(), relPart); } @@ -231,8 +236,7 @@ public final class PackageRelationshipCollection implements PackageRelationship rel = new PackageRelationship(container, sourcePart, targetUri, targetMode, relationshipType, id); - relationshipsByID.put(rel.getId(), rel); - relationshipsByType.put(rel.getRelationshipType(), rel); + addRelationship(rel);; if (targetMode == TargetMode.INTERNAL){ internalRelationshipsByTargetName.put(targetUri.toASCIIString(), rel); } diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFHyperlink.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFHyperlink.java index e806861..ef44115 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFHyperlink.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFHyperlink.java @@ -54,10 +54,17 @@ public class XSLFHyperlink implements Hyperlink<XSLFShape,XSLFTextParagraph> { if (id == null || id.isEmpty()) { return _link.getAction(); } - - URI targetURI = _sheet.getPackagePart().getRelationship(id).getTargetURI(); + String url=null; + PackagePart pp = _sheet.getPackagePart(); + if (pp!=null) { + PackageRelationship rs = pp.getRelationship(id); + if (rs!=null) { + URI targetURI = rs.getTargetURI(); + url=targetURI.toASCIIString(); + } + } - return targetURI.toASCIIString(); + return url; } @Override diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureShape.java index da3cf45..865ca58 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureShape.java @@ -201,6 +201,10 @@ public class XSLFPictureShape extends XSLFSimpleShape XSLFPictureShape p = (XSLFPictureShape)sh; String blipId = p.getBlipId(); + if (blipId==null) { + // should we do anything about this? + return; + } String relId = getSheet().importBlip(blipId, p.getSheet().getPackagePart()); CTPicture ct = (CTPicture)getXmlObject(); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java index 8961700..5e6b427 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java @@ -591,7 +591,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> { PackagePart blipPart; try { blipPart = packagePart.getRelatedPart(blipRel); - } catch (InvalidFormatException e){ + } catch (Exception e){ throw new POIXMLException(e); } XSLFPictureData data = new XSLFPictureData(blipPart); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java index 3f686ec..1711e4d 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java @@ -41,6 +41,7 @@ import org.openxmlformats.schemas.drawingml.x2006.main.CTRegularTextRun; import org.openxmlformats.schemas.drawingml.x2006.main.CTSchemeColor; import org.openxmlformats.schemas.drawingml.x2006.main.CTShapeStyle; import org.openxmlformats.schemas.drawingml.x2006.main.CTSolidColorFillProperties; +import org.openxmlformats.schemas.drawingml.x2006.main.CTTextBodyProperties; import org.openxmlformats.schemas.drawingml.x2006.main.CTTextCharacterProperties; import org.openxmlformats.schemas.drawingml.x2006.main.CTTextField; import org.openxmlformats.schemas.drawingml.x2006.main.CTTextFont; @@ -206,9 +207,18 @@ public class XSLFTextRun implements TextRun { @Override public Double getFontSize(){ double scale = 1; - CTTextNormalAutofit afit = getParentParagraph().getParentShape().getTextBodyPr().getNormAutofit(); - if(afit != null) { - scale = (double)afit.getFontScale() / 100000; + XSLFTextParagraph pp = getParentParagraph(); + if (pp!=null) { + XSLFTextShape ps = pp.getParentShape(); + if (ps!=null) { + CTTextBodyProperties tbp = ps.getTextBodyPr(); + if (tbp!=null) { + CTTextNormalAutofit afit = tbp.getNormAutofit(); + if(afit != null) { + scale = (double)afit.getFontScale() / 100000; + } + } + } } CharacterPropertyFetcher<Double> fetcher = new CharacterPropertyFetcher<Double>(_p.getIndentLevel()){ @@ -222,7 +232,8 @@ public class XSLFTextRun implements TextRun { } }; fetchCharacterProperty(fetcher); - return fetcher.getValue() == null ? null : fetcher.getValue()*scale; + Double result=fetcher.getValue() == null ? null : fetcher.getValue()*scale; + return result; } /** @@ -586,10 +597,12 @@ public class XSLFTextRun implements TextRun { PaintStyle srcFontColor = r.getFontColor(); if(srcFontColor != null && !srcFontColor.equals(getFontColor())){ - setFontColor(srcFontColor); + // setFontColor will throw IllegalArgumentException if this is not checked + if (srcFontColor instanceof SolidPaint) + setFontColor(srcFontColor); } - double srcFontSize = r.getFontSize(); + Double srcFontSize=r.getFontSize(); if(srcFontSize != getFontSize()){ setFontSize(srcFontSize); } diff --git a/src/ooxml/testcases/org/apache/poi/sl/TestFonts.java b/src/ooxml/testcases/org/apache/poi/sl/TestFonts.java index 099813c..b7a6d16 100644 --- a/src/ooxml/testcases/org/apache/poi/sl/TestFonts.java +++ b/src/ooxml/testcases/org/apache/poi/sl/TestFonts.java @@ -79,7 +79,7 @@ public class TestFonts { 348, // Windows 10, 15.6" 3840x2160 362, // Windows 10, 13.3" 1080p high-dpi 372, // Ubuntu Xenial, 15", 1680x1050 - 377, 398, 399, // Mac + 377, 391, 398, 399, // Mac 406 // Ubuntu Xenial, 15", 1680x1050 };
Applied the modified patch via r1831974 I couldn't import the diff via IntelliJ - not sure how you generated it - have you called "git diff" for each file separately? Furthermore please provide further patches as attachment and not inline in your message. I leave the issue open, as I guess there will be more coming ...
We seem to have fixed the problem reported here, so I'd rather close this for now. Please report new bug-entries if there are still things that need fixing.