From: Nick Burch <nick@apache.org>
Date: Sat, 15 Mar 2014 08:16:43 +0000 (+0000)
Subject: When looking for text run related records after a TextHeaderAtom, provide a cleaner... 
X-Git-Tag: REL_3_11_BETA1~215
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=80caa1ddcfa1dacd041d7790e621d94696759e22;p=poi.git

When looking for text run related records after a TextHeaderAtom, provide a cleaner way to skip ones we don't care about, and a cleaner way to find the StyleTextPropAtom. Should fix #56260

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

diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/Sheet.java b/src/scratchpad/src/org/apache/poi/hslf/model/Sheet.java
index 875b2dfafa..e912edf528 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/model/Sheet.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/model/Sheet.java
@@ -29,6 +29,7 @@ import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.hslf.record.CString;
 import org.apache.poi.hslf.record.ColorSchemeAtom;
 import org.apache.poi.hslf.record.EscherTextboxWrapper;
+import org.apache.poi.hslf.record.MasterTextPropAtom;
 import org.apache.poi.hslf.record.OEPlaceholderAtom;
 import org.apache.poi.hslf.record.PPDrawing;
 import org.apache.poi.hslf.record.Record;
@@ -42,6 +43,7 @@ import org.apache.poi.hslf.record.TextBytesAtom;
 import org.apache.poi.hslf.record.TextCharsAtom;
 import org.apache.poi.hslf.record.TextHeaderAtom;
 import org.apache.poi.hslf.record.TextRulerAtom;
+import org.apache.poi.hslf.record.TextSpecInfoAtom;
 import org.apache.poi.hslf.usermodel.SlideShow;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -197,29 +199,35 @@ public abstract class Sheet {
                 StyleTextPropAtom stpa = null;
                 TextRun trun = null;
                 Record next = null;
+                Record subs = null;
                 
-                // Is there a StyleTextPropAtom after the Text Atom?
-                // TODO Do we need to check for text ones two away as well?
-                // TODO Refactor this to happen later in this for loop
-                if (i < (records.length - 2)) {
-                    next = records[i+2];
-                    if (next instanceof StyleTextPropAtom) {
-                        stpa = (StyleTextPropAtom)next;
-                    }
-                }
-
                 // See what follows the TextHeaderAtom
                 next = records[i+1];
-                
-                // Is it one we ignore and check the one after that?
                 if (i < records.length - 2) {
-                    // TODO MasterTextPropAtom
-                    if (next instanceof TextRulerAtom) {
-                        next = records[i+2];
+                    subs = records[i+2];
+                }
+                
+                // Is the next record one we need to skip over?
+                if (subs != null) {
+                    if (next instanceof TextRulerAtom ||
+                        next instanceof MasterTextPropAtom ||
+                        next instanceof TextSpecInfoAtom) {
+                        // Ignore this one, check the one after
+                        next = subs;
+                        if (i < records.length - 3) {
+                            subs = records[i+3];
+                        } else {
+                            subs = null;
+                        }
                     }
                 }
                 
-                // Is it one we need to record?
+                // Is the subsequent record a style one?
+                if (subs != null && subs instanceof StyleTextPropAtom) {
+                    stpa = (StyleTextPropAtom)subs;
+                }
+                
+                // Now, check if the next record is one to record
                 if (next instanceof TextCharsAtom) {
                     TextCharsAtom tca = (TextCharsAtom)next;
                     trun = new TextRun(tha, tca, stpa);
@@ -233,9 +241,6 @@ public abstract class Sheet {
                     //  text. Only the header remains, which isn't useful alone 
                     // Skip on to the next TextHeaderAtom
                     continue;
-                } else if (next.getRecordType() == (long)RecordTypes.TextSpecInfoAtom.typeID ||
-                           next.getRecordType() == (long)RecordTypes.BaseTextPropAtom.typeID) { 
-                    // Safe to ignore these ones
                 } else {
                     logger.log(POILogger.ERROR, "Found a TextHeaderAtom not followed by a TextBytesAtom or TextCharsAtom: Followed by " + next.getRecordType());
                 }
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 efc196d3f7..0372472632 100644
--- a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
+++ b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
@@ -331,9 +331,18 @@ public final class TestBugs {
 
         assertTrue("No Exceptions while reading file", true);
 
+        // Check the first slide
         Slide slide = ppt.getSlides()[0];
-        TextRun[] tr1 = slide.getTextRuns();
-
+        TextRun[] slTr = slide.getTextRuns();
+        
+        // Has two text runs, one from slide text, one from drawing
+        assertEquals(2, slTr.length);
+        assertEquals(false, slTr[0].isDrawingBased());
+        assertEquals(true, slTr[1].isDrawingBased());
+        assertEquals("First run", slTr[0].getText());
+        assertEquals("Second run", slTr[1].getText());
+
+        // Check the shape based text runs
         List<TextRun> lst = new ArrayList<TextRun>();
         Shape[] shape = slide.getShapes();
         for (int i = 0; i < shape.length; i++) {
@@ -345,13 +354,11 @@ public final class TestBugs {
             }
 
         }
-        TextRun[] tr2 = new TextRun[lst.size()];
-        lst.toArray(tr2);
-
-        assertEquals(tr1.length, tr2.length);
-        for (int i = 0; i < tr1.length; i++) {
-            assertEquals(tr1[i].getText(), tr2[i].getText());
-        }
+        // There should be only one shape based one found
+        assertEquals(1, lst.size());
+        
+        // And it should be the second one
+        assertEquals("Second run", lst.get(0).getText());
     }
 
     /**