Fixed some style related corner cases and adapted the tests for it

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

src/scratchpad/src/org/apache/poi/hslf/model/textproperties/BitMaskTextProp.java

@@ -17,6 +17,10 @@
 
 package org.apache.poi.hslf.model.textproperties;
 
+import org.apache.poi.hslf.record.Record;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
+
 /** 
  * Definition of a special kind of property of some text, or its 
  *  paragraph. For these properties, a flag in the "contains" header 
@@ -25,6 +29,8 @@ package org.apache.poi.hslf.model.textproperties;
  *  (but related) properties
  */
 public abstract class BitMaskTextProp extends TextProp implements Cloneable {
+    protected static final POILogger logger = POILogFactory.getLogger(BitMaskTextProp.class);
+    
     private String[] subPropNames;
 	private int[] subPropMasks;
 	private boolean[] subPropMatches;
@@ -34,22 +40,25 @@ public abstract class BitMaskTextProp extends TextProp implements Cloneable {
 	/** Fetch the list of if the sub properties match or not */
 	public boolean[] getSubPropMatches() { return subPropMatches; }
 
-	public BitMaskTextProp(int sizeOfDataBlock, int maskInHeader, String overallName, String[] subPropNames) {
+	protected BitMaskTextProp(int sizeOfDataBlock, int maskInHeader, String overallName, String... subPropNames) {
 		super(sizeOfDataBlock,maskInHeader,"bitmask");
 		this.subPropNames = subPropNames;
 		this.propName = overallName;
 		subPropMasks = new int[subPropNames.length];
 		subPropMatches = new boolean[subPropNames.length];
 		
+		int LSB = Integer.lowestOneBit(maskInHeader);
+		
 		// Initialise the masks list
 		for(int i=0; i<subPropMasks.length; i++) {
-			subPropMasks[i] = (1 << i);
+			subPropMasks[i] = (LSB << i);
 		}
 	}
 	
 	/**
 	 * Calculate mask from the subPropMatches.
 	 */
+	@Override
 	public int getWriteMask() {
 	    /*
 	     * The dataValue can't be taken as a mask, as sometimes certain properties
@@ -63,17 +72,39 @@ public abstract class BitMaskTextProp extends TextProp implements Cloneable {
 		return mask;
 	}
 
-	public void setWriteMask(int containsField) {
+	/**
+	 * Sets the write mask, i.e. which defines the text properties to be considered
+	 *
+	 * @param writeMask the mask, bit values outside the property mask range will be ignored
+	 */
+	public void setWriteMask(int writeMask) {
         int i = 0;
         for (int subMask : subPropMasks) {
-            if ((containsField & subMask) != 0) subPropMatches[i] = true;
+            subPropMatches[i++] = ((writeMask & subMask) != 0);
-            i++;
         }
 	}
 
+	/**
+	 * Return the text property value.
+	 * Clears all bits of the value, which are marked as unset.
+	 * 
+	 * @return the text property value.
+	 */
+	@Override
+	public int getValue() {
+	    int val = dataValue, i = 0;;
+	    for (int mask : subPropMasks) {
+	        if (!subPropMatches[i++]) {
+	            val &= ~mask;
+	        }
+	    }
+	    return val;
+	}
+	
+	
 	/**
 	 * Set the value of the text property, and recompute the sub
-	 * properties based on it, i.e. all unset subvalues won't be saved.
+	 * properties based on it, i.e. all unset subvalues will be cleared.
 	 * Use {@link #setSubValue(boolean, int)} to explicitly set subvalues to {@code false}. 
 	 */
 	@Override
@@ -87,11 +118,37 @@ public abstract class BitMaskTextProp extends TextProp implements Cloneable {
 		}
 	}
 
+	/**
+	 * Convenience method to set a value with mask, without splitting it into the subvalues
+	 *
+	 * @param val
+	 * @param writeMask
+	 */
+	public void setValueWithMask(int val, int writeMask) {
+	    setWriteMask(writeMask);
+	    dataValue = val;
+	    dataValue = getValue();
+	    if (val != dataValue) {
+	        logger.log(POILogger.WARN, "Style properties of '"+getName()+"' don't match mask - output will be sanitized");
+	        if (logger.check(POILogger.DEBUG)) {
+	            StringBuilder sb = new StringBuilder("The following style attributes of the '"+getName()+"' property will be ignored:\n");
+	            int i=0;
+	            for (int mask : subPropMasks) {
+	                if (!subPropMatches[i] && (val & mask) != 0) {
+	                    sb.append(subPropNames[i]+",");
+	                }
+	                i++;
+	            }
+	            logger.log(POILogger.DEBUG, sb.toString());
+	        }
+	    }
+	}
+	
 	/**
 	 * Fetch the true/false status of the subproperty with the given index
 	 */
 	public boolean getSubValue(int idx) {
-		return (dataValue & subPropMasks[idx]) != 0;
+		return subPropMatches[idx] && ((dataValue & subPropMasks[idx]) != 0);
 	}
 
 	/**

src/scratchpad/src/org/apache/poi/hslf/model/textproperties/CharFlagsTextProp.java

@@ -34,7 +34,7 @@ public class CharFlagsTextProp extends BitMaskTextProp {
 
     public static final String NAME = "char_flags";
 	public CharFlagsTextProp() {
-		super(2, 0xffff, NAME, new String[] {
+		super(2, 0xffff, NAME,
 			"bold",                 // 0x0001  A bit that specifies whether the characters are bold.
 			"italic",               // 0x0002  A bit that specifies whether the characters are italicized.
 			"underline",            // 0x0004  A bit that specifies whether the characters are underlined.
@@ -50,8 +50,7 @@ public class CharFlagsTextProp extends BitMaskTextProp {
             "pp9rt_3",              // 0x1000
             "pp9rt_4",              // 0x2000
             "unused4_1",            // 0x4000  Undefined and MUST be ignored.
-                "unused4_2",            // 0x8000  Undefined and MUST be ignored.
+            "unused4_2"             // 0x8000  Undefined and MUST be ignored.
-			}
 		);
 	}
 }

src/scratchpad/src/org/apache/poi/hslf/model/textproperties/ParagraphFlagsTextProp.java

@@ -31,11 +31,11 @@ public final class ParagraphFlagsTextProp extends BitMaskTextProp {
     public static final String NAME = "paragraph_flags";
 
 	public ParagraphFlagsTextProp() {
-		super(2,  0xF, NAME, new String[] {
+		super(2,  0xF, NAME,
 			"bullet",
             "bullet.hardfont",
 			"bullet.hardcolor",
-                    "bullet.hardsize"}
+            "bullet.hardsize"
 		);
 	}
 }

src/scratchpad/src/org/apache/poi/hslf/model/textproperties/TextPropCollection.java

@@ -87,7 +87,7 @@ public class TextPropCollection {
         new TextProp(2, 0x8000, "defaultTabSize"),
         new TabStopPropCollection(), // tabstops size is variable!
         new FontAlignmentProp(),
-        new TextProp(2, 0xE0000, "wrapFlags"), // charWrap | wordWrap | overflow
+        new WrapFlagsTextProp(),
         new TextProp(2, 0x200000, "textDirection"),
         // 0x400000 MUST be zero and MUST be ignored
         new TextProp(0, 0x800000, "bullet.blip"), // TODO: check size
@@ -266,9 +266,11 @@ public class TextPropCollection {
                     maskSpecial |= tp.getMask();
                     continue;
                 }
-				prop.setValue(val);
+				
 				if (prop instanceof BitMaskTextProp) {
-				    ((BitMaskTextProp)prop).setWriteMask(containsField);
+				    ((BitMaskTextProp)prop).setValueWithMask(val, containsField);
+				} else {
+				    prop.setValue(val);
 				}
 				bytesPassed += prop.getSize();
 				addProp(prop);
@@ -318,13 +320,7 @@ public class TextPropCollection {
 		// Then the mask field
 		int mask = maskSpecial;
 		for (TextProp textProp : textPropList) {
-            // sometimes header indicates that the bitmask is present but its value is 0
-            if (textProp instanceof BitMaskTextProp) {
-                if (mask == 0) mask |= textProp.getWriteMask();
-            }
-            else {
             mask |= textProp.getWriteMask();
-            }
         }
 		StyleTextPropAtom.writeLittleEndian(mask,o);
 
@@ -399,6 +395,9 @@ public class TextPropCollection {
         StringBuilder out = new StringBuilder();
         out.append("  chars covered: " + getCharactersCovered());
         out.append("  special mask flags: 0x" + HexDump.toHex(getSpecialMask()) + "\n");
+        if (textPropType == TextPropType.paragraph) {
+            out.append("  indent level: "+getIndentLevel()+"\n");
+        }
         for(TextProp p : getTextPropList()) {
             out.append("    " + p.getName() + " = " + p.getValue() );
             out.append(" (0x" + HexDump.toHex(p.getValue()) + ")\n");

src/scratchpad/src/org/apache/poi/hslf/model/textproperties/WrapFlagsTextProp.java

@@ -0,0 +1,30 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hslf.model.textproperties;
+
+public class WrapFlagsTextProp extends BitMaskTextProp {
+    public static final int CHAR_WRAP_IDX = 0;
+    public static final int WORD_WRAO_IDX = 1;
+    public static final int OVERFLOW_IDX = 2;
+    
+    public static final String NAME = "wrapFlags";
+    
+    public WrapFlagsTextProp() {
+        super(2, 0xE0000, NAME, "charWrap", "wordWrap", "overflow");
+    }
+}

src/scratchpad/src/org/apache/poi/hslf/record/StyleTextPropAtom.java

@@ -310,15 +310,11 @@ public final class StyleTextPropAtom extends RecordAtom
     
             // First up, we need to serialise the paragraph properties
             for(TextPropCollection tpc : paragraphStyles) {
-                // ensure, that the paragraphs flags exist, no matter if anthing is set
-                tpc.addWithName(ParagraphFlagsTextProp.NAME);
                 tpc.writeOut(baos);
             }
     
             // Now, we do the character ones
             for(TextPropCollection tpc : charStyles) {
-                // ditto for the char flags
-                // tpc.addWithName(CharFlagsTextProp.NAME);
                 tpc.writeOut(baos);
             }
     

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java

@@ -398,9 +398,11 @@ public final class HSLFSlideShow implements SlideShow {
 		// Finally, generate model objects for everything
 		// Notes first
 		for (org.apache.poi.hslf.record.Notes n : notesRecords) {
-		    if (n == null) continue;
+		    HSLFNotes hn = null;
-		    HSLFNotes hn = new HSLFNotes(n);
+		    if (n != null) {
+    		    hn = new HSLFNotes(n);
     		    hn.setSlideShow(this);
+		    }
 		    _notes.add(hn);
 		}
 		// Then slides

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextRun.java

@@ -112,9 +112,9 @@ public final class HSLFTextRun implements TextRun {
 		BitMaskTextProp prop = (BitMaskTextProp)characterStyle.findByName(CharFlagsTextProp.NAME);
 
 		if (prop == null){
+            int txtype = parentParagraph.getRunType();
 			HSLFSheet sheet = parentParagraph.getSheet();
 			if (sheet != null) {
-				int txtype = parentParagraph.getParentShape().getRunType();
 				HSLFMasterSheet master = sheet.getMasterSheet();
 				if (master != null){
 					prop = (BitMaskTextProp)master.getStyleAttribute(txtype, parentParagraph.getIndentLevel(), CharFlagsTextProp.NAME, true);

src/scratchpad/testcases/org/apache/poi/hslf/record/TestStyleTextPropAtom.java

@@ -17,23 +17,22 @@
 
 package org.apache.poi.hslf.record;
 
-import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.*;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.List;
 
-import junit.framework.TestCase;
-
 import org.apache.poi.hslf.model.textproperties.*;
 import org.apache.poi.util.HexDump;
+import org.junit.Test;
 
 /**
  * Tests that StyleTextPropAtom works properly
  *
  * @author Nick Burch (nick at torchbox dot com)
  */
-public final class TestStyleTextPropAtom extends TestCase {
+public final class TestStyleTextPropAtom {
     /** From a real file: a paragraph with 4 different styles */
     private static final byte[] data_a = new byte[] {
       0, 0, 0xA1-256, 0x0F, 0x2A, 0, 0, 0,
@@ -138,6 +137,7 @@ public final class TestStyleTextPropAtom extends TestCase {
     };
     private static final int data_d_text_len = 0xA0-1;
 
+    @Test
     public void testRecordType() {
         StyleTextPropAtom stpa = new StyleTextPropAtom(data_a,0,data_a.length);
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
@@ -148,6 +148,7 @@ public final class TestStyleTextPropAtom extends TestCase {
     }
 
 
+    @Test
     public void testCharacterStyleCounts() {
         StyleTextPropAtom stpa = new StyleTextPropAtom(data_a,0,data_a.length);
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
@@ -162,6 +163,7 @@ public final class TestStyleTextPropAtom extends TestCase {
         assertEquals(5, stpb.getCharacterStyles().size());
     }
 
+    @Test
     public void testParagraphStyleCounts() {
         StyleTextPropAtom stpa = new StyleTextPropAtom(data_a,0,data_a.length);
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
@@ -177,6 +179,7 @@ public final class TestStyleTextPropAtom extends TestCase {
     }
 
 
+    @Test
     public void testCharacterStyleLengths() {
         StyleTextPropAtom stpa = new StyleTextPropAtom(data_a,0,data_a.length);
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
@@ -207,6 +210,7 @@ public final class TestStyleTextPropAtom extends TestCase {
     }
 
 
+    @Test
     public void testCharacterPropOrdering() {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
         stpb.setParentTextSize(data_b_text_len);
@@ -254,6 +258,7 @@ public final class TestStyleTextPropAtom extends TestCase {
         assertEquals(24, tp_4_3.getValue());
     }
 
+    @Test
     public void testParagraphProps() {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
         stpb.setParentTextSize(data_b_text_len);
@@ -298,6 +303,7 @@ public final class TestStyleTextPropAtom extends TestCase {
         assertEquals(80, tp_4_2.getValue());
     }
 
+    @Test
     public void testCharacterProps() {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
         stpb.setParentTextSize(data_b_text_len);
@@ -369,6 +375,8 @@ public final class TestStyleTextPropAtom extends TestCase {
         assertEquals(0x0003, cf_4_1.getValue());
     }
 
+    @SuppressWarnings("unused")
+    @Test
     public void testFindAddTextProp() {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data_b,0,data_b.length);
         stpb.setParentTextSize(data_b_text_len);
@@ -423,6 +431,7 @@ public final class TestStyleTextPropAtom extends TestCase {
      * Try to recreate an existing StyleTextPropAtom (a) from the empty
      *  constructor, and setting the required properties
      */
+    @Test
     public void testCreateAFromScatch() throws Exception {
         // Start with an empty one
         StyleTextPropAtom stpa = new StyleTextPropAtom(54);
@@ -460,6 +469,7 @@ public final class TestStyleTextPropAtom extends TestCase {
      * Try to recreate an existing StyleTextPropAtom (b) from the empty
      *  constructor, and setting the required properties
      */
+    @Test
     public void testCreateBFromScatch() throws Exception {
         // Start with an empty one
         StyleTextPropAtom stpa = new StyleTextPropAtom(data_b_text_len);
@@ -603,8 +613,8 @@ public final class TestStyleTextPropAtom extends TestCase {
                 ByteArrayOutputStream ba = new ByteArrayOutputStream();
                 ByteArrayOutputStream bb = new ByteArrayOutputStream();
 
-                ca.writeOut(ba, StyleTextPropAtom.characterTextPropTypes);
+                ca.writeOut(ba);
-                cb.writeOut(bb, StyleTextPropAtom.characterTextPropTypes);
+                cb.writeOut(bb);
                 byte[] cab = ba.toByteArray();
                 byte[] cbb = bb.toByteArray();
 
@@ -630,32 +640,46 @@ public final class TestStyleTextPropAtom extends TestCase {
         }
     }
 
+    @Test
     public void testWriteA() {
         doReadWrite(data_a, -1);
     }
 
+    @Test
     public void testLoadWriteA() {
         doReadWrite(data_b, data_b_text_len);
     }
 
 
+    @Test
     public void testWriteB() {
         doReadWrite(data_b, -1);
     }
 
+    @Test
     public void testLoadWriteB() {
         doReadWrite(data_b, data_b_text_len);
     }
 
+    @Test
     public void testLoadWriteC() {
-        doReadWrite(data_c, data_c_text_len);
+        // BitMaskTextProperties will sanitize the output
+        byte expected[] = data_c.clone();
+        expected[56] = 0;
+        expected[68] = 0;
+        doReadWrite(data_c, expected, data_c_text_len);
     }
 
+    @Test
     public void testLoadWriteD() {
         doReadWrite(data_d, data_d_text_len);
     }
 
     protected void doReadWrite(byte[] data, int textlen) {
+        doReadWrite(data, data, textlen);
+    }
+    
+    protected void doReadWrite(byte[] data, byte[] expected, int textlen) {
         StyleTextPropAtom stpb = new StyleTextPropAtom(data, 0,data.length);
         if(textlen != -1) stpb.setParentTextSize(textlen);
 
@@ -667,15 +691,16 @@ public final class TestStyleTextPropAtom extends TestCase {
         }
         byte[] bytes = out.toByteArray();
 
-        assertEquals(data.length, bytes.length);
+        assertEquals(expected.length, bytes.length);
         try {
-            assertArrayEquals(data, bytes);
+            assertArrayEquals(expected, bytes);
         } catch (Throwable e){
             //print hex dump if failed
-            assertEquals(HexDump.toHex(data), HexDump.toHex(bytes));
+            assertEquals(HexDump.toHex(expected), HexDump.toHex(bytes));
         }
     }
 
+    @Test
     public void testNotEnoughDataProp() {
         // We don't have enough data in the record to cover
         //  all the properties the mask says we have
@@ -689,6 +714,7 @@ public final class TestStyleTextPropAtom extends TestCase {
     /**
      * Check the test data for Bug 40143.
      */
+   @Test
    public void testBug40143() {
         StyleTextPropAtom atom = new StyleTextPropAtom(data_d, 0, data_d.length);
         atom.setParentTextSize(data_d_text_len);
@@ -711,9 +737,11 @@ public final class TestStyleTextPropAtom extends TestCase {
     /**
      * Check the test data for Bug 42677.
      */
+     @Test
      public void test42677() {
         int length = 18;
-        byte[] data = {0x00, 0x00, (byte)0xA1, 0x0F, 0x28, 0x00, 0x00, 0x00,
+        byte[] data = {
+            0x00, 0x00, (byte)0xA1, 0x0F, 0x28, 0x00, 0x00, 0x00,
             0x13, 0x00 , 0x00 , 0x00 , 0x00 , 0x00 , (byte)0xF1 , 0x20 , 0x00, 0x00 , 0x00 , 0x00 ,
             0x22 , 0x20 , 0x00 , 0x00 , 0x64 , 0x00 , 0x00 , 0x00 , 0x00 , (byte)0xFF ,
             0x00 , 0x00 , 0x13 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00 , 0x63 , 0x00 ,
@@ -735,6 +763,7 @@ public final class TestStyleTextPropAtom extends TestCase {
      *   00 00 00 01 18 00 00 01 18 01 00 00 00 01 1C 00 00 01 1C
      * </StyleTextPropAtom>
      */
+     @Test
     public void test45815() {
         int length = 19;
         byte[] data = {
@@ -750,7 +779,13 @@ public final class TestStyleTextPropAtom extends TestCase {
                 0x01, 0x18, 0x01, 0x00, 0x00, 0x00, 0x01, 0x1C, 0x00, 0x00,
                 0x01, 0x1C
         };
-        doReadWrite(data, length);
+
+        // changed original data: ... 0x41 and 0x06 don't match
+        // the bitmask text properties will sanitize the bytes and thus the bytes differ
+        byte[] exptected = data.clone();
+        exptected[18] = 0;
+        
+        doReadWrite(data, exptected, length);
     }
 
 }