#45124 - inserting text or images wipes out boldness and makes everything italic

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711704 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 168 insertions, 120 deletions

diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/textproperties/TextPropCollection.java b/src/scratchpad/src/org/apache/poi/hslf/model/textproperties/TextPropCollection.java
index 84cfe9940f..08696b2187 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/model/textproperties/TextPropCollection.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/model/textproperties/TextPropCollection.java
@@ -20,6 +20,7 @@ package org.apache.poi.hslf.model.textproperties;
 import java.io.*;
 import java.util.*;
 
+import org.apache.poi.hslf.exceptions.HSLFException;
 import org.apache.poi.hslf.record.StyleTextPropAtom;
 import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
@@ -32,7 +33,7 @@ import org.apache.poi.util.LittleEndian;
  */
 public class TextPropCollection {
     /** All the different kinds of paragraph properties we might handle */
-    public static final TextProp[] paragraphTextPropTypes = {
+    private static final TextProp[] paragraphTextPropTypes = {
         // TextProp order is according to 2.9.20 TextPFException,
         // bitmask order can be different
         new ParagraphFlagsTextProp(),
@@ -60,7 +61,7 @@ public class TextPropCollection {
     };
     
     /** All the different kinds of character properties we might handle */
-    public static final TextProp[] characterTextPropTypes = new TextProp[] {
+    private static final TextProp[] characterTextPropTypes = new TextProp[] {
         new TextProp(0, 0x100000, "pp10ext"),
         new TextProp(0, 0x1000000, "newAsian.font.index"), // A bit that specifies whether the newEAFontRef field of the TextCFException10 structure that contains this CFMasks exists.
         new TextProp(0, 0x2000000, "cs.font.index"), // A bit that specifies whether the csFontRef field of the TextCFException10 structure that contains this CFMasks exists.
@@ -84,7 +85,7 @@ public class TextPropCollection {
     // indentLevel is only valid for paragraph collection
     // if it's set to -1, it must be omitted - see 2.9.36 TextMasterStyleLevel
     private short indentLevel = 0;
-	private final List<TextProp> textPropList = new ArrayList<TextProp>();
+	private final Map<String,TextProp> textProps = new HashMap<String,TextProp>();
     private int maskSpecial = 0;
     private final TextPropType textPropType;
     
@@ -98,101 +99,89 @@ public class TextPropCollection {
         this.textPropType = textPropType;
     }
 
-    public int getSpecialMask() { return maskSpecial; }
+    public int getSpecialMask() {
+        return maskSpecial;
+    }
 
 	/** Fetch the number of characters this styling applies to */
-	public int getCharactersCovered() { return charactersCovered; }
-	/** Fetch the TextProps that define this styling */
-	public List<TextProp> getTextPropList() { return textPropList; }
+	public int getCharactersCovered() {
+	    return charactersCovered;
+    }
+
+	/** Fetch the TextProps that define this styling in the record order */
+	public List<TextProp> getTextPropList() {
+	    List<TextProp> orderedList = new ArrayList<TextProp>();
+        for (TextProp potProp : getPotentialProperties()) {
+            TextProp textProp = textProps.get(potProp.getName());
+            if (textProp != null) {
+                orderedList.add(textProp);
+            }
+        }
+	    return orderedList;
+    }
 	
 	/** Fetch the TextProp with this name, or null if it isn't present */
-	public TextProp findByName(String textPropName) {
-		for(TextProp prop : textPropList) {
-			if(prop.getName().equals(textPropName)) {
-				return prop;
-			}
-		}
-		return null;
+	public final TextProp findByName(String textPropName) {
+		return textProps.get(textPropName);
 	}
 
-	public TextProp removeByName(String name) {
-	    Iterator<TextProp> iter = textPropList.iterator();
-	    TextProp tp = null;
-	    while (iter.hasNext()) {
-	        tp = iter.next();
-	        if (tp.getName().equals(name)){
-	            iter.remove();
-	            break;
-	        }
-	    }
-	    return tp;
+	public final TextProp removeByName(String name) {
+	    return textProps.remove(name);
 	}
 	
-	/** Add the TextProp with this name to the list */
-	public TextProp addWithName(String name) {
-		// Find the base TextProp to base on
-		TextProp existing = findByName(name);
-		if (existing != null) return existing;
-		
-		TextProp base = null;
-		for (TextProp tp : getPotentialProperties()) {
-		    if (tp.getName().equals(name)) {
-		        base = tp;
-		        break;
-		    }
-		}
-		
-		if(base == null) {
-			throw new IllegalArgumentException("No TextProp with name " + name + " is defined to add from. "
-		        + "Character and paragraphs have their own properties/names.");
-		}
-		
-		// Add a copy of this property, in the right place to the list
-		TextProp textProp = base.clone();
-		addProp(textProp);
-		return textProp;
-	}
-
-	public TextPropType getTextPropType() {
+	public final TextPropType getTextPropType() {
 	    return textPropType;
 	}
 	
 	private TextProp[] getPotentialProperties() {
 	    return (textPropType == TextPropType.paragraph) ? paragraphTextPropTypes : characterTextPropTypes;
 	}
+
+	/**
+	 * Checks the paragraph or character properties for the given property name.
+	 * Throws a HSLFException, if the name doesn't belong into this set of properties 
+	 *
+	 * @param name the property name
+	 * @return if found, the property template to copy from
+	 */
+	private TextProp validatePropName(String name) {
+       for (TextProp tp : getPotentialProperties()) {
+            if (tp.getName().equals(name)) {
+                return tp;
+            }
+        }
+       String errStr = 
+           "No TextProp with name " + name + " is defined to add from. " +
+           "Character and paragraphs have their own properties/names.";
+       throw new HSLFException(errStr);       
+	}
 	
+    /** Add the TextProp with this name to the list */
+    public final TextProp addWithName(String name) {
+        // Find the base TextProp to base on
+        TextProp existing = findByName(name);
+        if (existing != null) return existing;
+        
+        // Add a copy of this property
+        TextProp textProp = validatePropName(name).clone();
+        textProps.put(name,textProp);
+        return textProp;
+    }
+
 	/**
 	 * Add the property at the correct position. Replaces an existing property with the same name.
 	 *
 	 * @param textProp the property to be added
 	 */
-	public void addProp(TextProp textProp) {
-	    assert(textProp != null);
-	    
-        int pos = 0;
-        boolean found = false;
-        for (TextProp curProp : getPotentialProperties()) {
-            String potName = curProp.getName();
-            if (pos == textPropList.size() || potName.equals(textProp.getName())) {
-                if (textPropList.size() > pos && potName.equals(textPropList.get(pos).getName())) {
-                    // replace existing prop (with same name)
-                    textPropList.set(pos, textProp);
-                } else {
-                    textPropList.add(pos, textProp);
-                }
-                found = true;
-                break;
-            }
-            
-            if (potName.equals(textPropList.get(pos).getName())) {
-                pos++;
-            }
-        }
+	public final void addProp(TextProp textProp) {
+	    if (textProp == null) {
+	        throw new HSLFException("TextProp must not be null");
+	    }
 
-        if(!found) {
-            String err = "TextProp with name " + textProp.getName() + " doesn't belong to this collection.";
-            throw new IllegalArgumentException(err);
-        }
+	    String propName = textProp.getName();
+	    validatePropName(propName);
+	    
+	    textProps.put(propName, textProp);
 	}
 
 	/**
@@ -254,8 +243,8 @@ public class TextPropCollection {
         this.charactersCovered = other.charactersCovered;
         this.indentLevel = other.indentLevel;
         this.maskSpecial = other.maskSpecial;
-        this.textPropList.clear();
-        for (TextProp tp : other.textPropList) {
+        this.textProps.clear();
+        for (TextProp tp : other.textProps.values()) {
             TextProp tpCopy = (tp instanceof BitMaskTextProp)
                 ? ((BitMaskTextProp)tp).cloneAll()
                 : tp.clone();
@@ -285,25 +274,22 @@ public class TextPropCollection {
 
 		// Then the mask field
 		int mask = maskSpecial;
-		for (TextProp textProp : textPropList) {
+		for (TextProp textProp : textProps.values()) {
             mask |= textProp.getWriteMask();
         }
 		StyleTextPropAtom.writeLittleEndian(mask,o);
 
 		// Then the contents of all the properties
-		for (TextProp potProp : getPotentialProperties()) {
-    		for(TextProp textProp : textPropList) {
-    		    if (!textProp.getName().equals(potProp.getName())) continue;
-                int val = textProp.getValue();
-                if (textProp instanceof BitMaskTextProp && textProp.getWriteMask() == 0) {
-                    // don't add empty properties, as they can't be recognized while reading
-                    continue;
-                } else if (textProp.getSize() == 2) {
-    				StyleTextPropAtom.writeLittleEndian((short)val,o);
-    			} else if (textProp.getSize() == 4) {
-    				StyleTextPropAtom.writeLittleEndian(val,o);
-    			}
-    		}
+		for (TextProp textProp : getTextPropList()) {
+            int val = textProp.getValue();
+            if (textProp instanceof BitMaskTextProp && textProp.getWriteMask() == 0) {
+                // don't add empty properties, as they can't be recognized while reading
+                continue;
+            } else if (textProp.getSize() == 2) {
+                StyleTextPropAtom.writeLittleEndian((short)val,o);
+            } else if (textProp.getSize() == 4) {
+                StyleTextPropAtom.writeLittleEndian(val,o);
+            }
 		}
 	}
 
@@ -324,7 +310,7 @@ public class TextPropCollection {
         result = prime * result + charactersCovered;
         result = prime * result + maskSpecial;
         result = prime * result + indentLevel;
-        result = prime * result + ((textPropList == null) ? 0 : textPropList.hashCode());
+        result = prime * result + ((textProps == null) ? 0 : textProps.hashCode());
         return result;
     }
     /**
@@ -340,21 +326,7 @@ public class TextPropCollection {
             return false;
         }
 
-        if (textPropList == null) {
-            return (o.textPropList == null);
-        }        
-        
-        Map<String,TextProp> m = new HashMap<String,TextProp>();
-        for (TextProp tp : o.textPropList) {
-            m.put(tp.getName(), tp);
-        }
-        
-        for (TextProp tp : this.textPropList) {
-            TextProp otp = m.get(tp.getName());
-            if (!tp.equals(otp)) return false;
-        }
-        
-        return true;
+        return textProps.equals(o.textProps);
     }
 
     public String toString() {
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
index 79ca35caf7..bc8ac93c6e 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
@@ -26,6 +26,7 @@ import java.util.Arrays;
 import java.util.Iterator;

 import java.util.List;

 

+import org.apache.poi.hslf.exceptions.HSLFException;

 import org.apache.poi.hslf.model.PPFont;

 import org.apache.poi.hslf.model.textproperties.BitMaskTextProp;

 import org.apache.poi.hslf.model.textproperties.FontAlignmentProp;

@@ -467,6 +468,28 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
     }

 

     @Override

+    public void setBulletStyle(Object... styles) {

+        if (styles.length == 0) {

+            setBullet(false);

+        } else {

+            setBullet(true);

+            for (Object ostyle : styles) {

+                if (ostyle instanceof Number) {

+                    setBulletSize(((Number)ostyle).doubleValue());

+                } else if (ostyle instanceof Color) {

+                    setBulletColor((Color)ostyle);

+                } else if (ostyle instanceof Character) {

+                    setBulletChar((Character)ostyle);

+                } else if (ostyle instanceof String) {

+                    setBulletFont((String)ostyle);

+                } else if (ostyle instanceof AutoNumberingScheme) {

+                    throw new HSLFException("setting bullet auto-numberin scheme for HSLF not supported ... yet");

+                }

+            }

+        }

+    }

+    

+    @Override

     public HSLFTextShape getParentShape() {

         return _parentShape;

     }

@@ -535,6 +558,7 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
     public void setBulletColor(Color color) {

         Integer val = (color == null) ? null : new Color(color.getBlue(), color.getGreen(), color.getRed(), 254).getRGB();

         setParagraphTextPropVal("bullet.color", val);

+        setFlag(ParagraphFlagsTextProp.BULLET_HARDCOLOR_IDX, (color != null));

     }

 

     /**

@@ -542,7 +566,8 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
      */

     public Color getBulletColor() {

         TextProp tp = getPropVal(_paragraphStyle, "bullet.color", this);

-        if (tp == null) {

+        boolean hasColor = getFlag(ParagraphFlagsTextProp.BULLET_HARDCOLOR_IDX);

+        if (tp == null || !hasColor) {

             // if bullet color is undefined, return color of first run

             if (_runs.isEmpty()) return null;

             SolidPaint sp = _runs.get(0).getFontColor();

@@ -573,7 +598,8 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
      */

     public String getBulletFont() {

         TextProp tp = getPropVal(_paragraphStyle, "bullet.font", this);

-        if (tp == null) return getDefaultFontFamily();

+        boolean hasFont = getFlag(ParagraphFlagsTextProp.BULLET_HARDFONT_IDX);

+        if (tp == null || !hasFont) return getDefaultFontFamily();

         PPFont ppFont = getSheet().getSlideShow().getFont(tp.getValue());

         assert(ppFont != null);

         return ppFont.getFontName();

diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/record/TestStyleTextPropAtom.java b/src/scratchpad/testcases/org/apache/poi/hslf/record/TestStyleTextPropAtom.java
index e868aa6768..15877fdcf3 100644
--- a/src/scratchpad/testcases/org/apache/poi/hslf/record/TestStyleTextPropAtom.java
+++ b/src/scratchpad/testcases/org/apache/poi/hslf/record/TestStyleTextPropAtom.java
@@ -23,6 +23,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.List;
 
+import org.apache.poi.hslf.exceptions.HSLFException;
 import org.apache.poi.hslf.model.textproperties.*;
 import org.apache.poi.util.HexDump;
 import org.junit.Test;
@@ -375,8 +376,7 @@ public final class TestStyleTextPropAtom {
         assertEquals(0x0003, cf_4_1.getValue());
     }
 
-    @SuppressWarnings("unused")
-    @Test
+    @Test(expected=HSLFException.class)
     public void testFindAddTextProp() {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
         stpb.setParentTextSize(data_b_text_len);
@@ -392,6 +392,16 @@ public final class TestStyleTextPropAtom {
         TextPropCollection b_ch_2 = b_ch_l.get(1);
         TextPropCollection b_ch_3 = b_ch_l.get(2);
         TextPropCollection b_ch_4 = b_ch_l.get(3);
+        
+        assertNotNull(b_p_1);
+        assertNotNull(b_p_2);
+        assertNotNull(b_p_3);
+        assertNotNull(b_p_4);
+        
+        assertNotNull(b_ch_1);
+        assertNotNull(b_ch_2);
+        assertNotNull(b_ch_3);
+        assertNotNull(b_ch_4);
 
         // CharFlagsTextProp: 3 doesn't have, 4 does
         assertNull(b_ch_3.findByName("char_flags"));
@@ -419,12 +429,7 @@ public final class TestStyleTextPropAtom {
         assertEquals(new_sa, b_p_2.getTextPropList().get(2));
 
         // Check we get an error with a made up one
-        try {
-            b_p_2.addWithName("madeUpOne");
-            fail();
-        } catch(IllegalArgumentException e) {
-            // Good, as expected
-        }
+        b_p_2.addWithName("madeUpOne");
     }
 
     /**
diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
index bb150f7fda..d18e8a26d3 100644
--- a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
+++ b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
@@ -22,9 +22,11 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import java.awt.Color;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
@@ -47,10 +49,18 @@ import org.apache.poi.hslf.record.Document;
 import org.apache.poi.hslf.record.Record;
 import org.apache.poi.hslf.record.SlideListWithText;
 import org.apache.poi.hslf.record.SlideListWithText.SlideAtomsSet;
+import org.apache.poi.hslf.record.StyleTextPropAtom;
 import org.apache.poi.hslf.record.TextHeaderAtom;
+import org.apache.poi.sl.draw.DrawPaint;
+import org.apache.poi.sl.usermodel.PaintStyle;
+import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
 import org.apache.poi.sl.usermodel.PictureData.PictureType;
+import org.apache.poi.sl.usermodel.Slide;
 import org.apache.poi.sl.usermodel.SlideShow;
 import org.apache.poi.sl.usermodel.SlideShowFactory;
+import org.apache.poi.sl.usermodel.TextBox;
+import org.apache.poi.sl.usermodel.TextParagraph;
+import org.apache.poi.sl.usermodel.TextRun;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.StringUtil;
 import org.apache.poi.util.Units;
@@ -642,6 +652,41 @@ public final class TestBugs {
     
     @Test
     public void bug58516() throws IOException {
-        SlideShowFactory.create(_slTests.getFile("bug58516.ppt"));
+        SlideShowFactory.create(_slTests.getFile("bug58516.ppt")).close();
+    }
+
+    @Test
+    public void bug45124() throws IOException {
+        SlideShow<?,?> ppt = SlideShowFactory.create(_slTests.getFile("bug45124.ppt"));
+        Slide<?,?> slide1 = ppt.getSlides().get(1);
+
+        TextBox<?,?> res = slide1.createTextBox();
+        res.setAnchor(new java.awt.Rectangle(60, 150, 700, 100));
+        res.setText("I am italic-false, bold-true inserted text");
+        
+
+        TextParagraph<?,?,?> tp = res.getTextParagraphs().get(0);
+        TextRun rt = tp.getTextRuns().get(0);
+        rt.setItalic(false);
+        assertTrue(rt.isBold());
+        
+        tp.setBulletStyle(Color.red, 'A');
+
+        SlideShow<?,?> ppt2 = HSLFTestDataSamples.writeOutAndReadBack((HSLFSlideShow)ppt);
+        ppt.close();
+        
+        res = (TextBox<?,?>)ppt2.getSlides().get(1).getShapes().get(1);
+        tp = res.getTextParagraphs().get(0);
+        rt = tp.getTextRuns().get(0);
+        
+        assertFalse(rt.isItalic());
+        assertTrue(rt.isBold());
+        PaintStyle ps = tp.getBulletStyle().getBulletFontColor();
+        assertTrue(ps instanceof SolidPaint);
+        Color actColor = DrawPaint.applyColorTransform(((SolidPaint)ps).getSolidColor());
+        assertEquals(Color.red, actColor);
+        assertEquals("A", tp.getBulletStyle().getBulletCharacter());
+        
+        ppt2.close();
     }
 }
diff --git a/test-data/slideshow/bug45124.ppt b/test-data/slideshow/bug45124.ppt
new file mode 100644
index 0000000000..2fd87b15e3
--- /dev/null
+++ b/test-data/slideshow/bug45124.ppt