Fixed white space handling during refinement, added test cases, corrected error in graphics handling

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

6 files changed, 149 insertions, 121 deletions

diff --git a/src/java/org/apache/fop/fo/InlineCharIterator.java b/src/java/org/apache/fop/fo/InlineCharIterator.java
index ed90d50c0..267b251d4 100644
--- a/src/java/org/apache/fop/fo/InlineCharIterator.java
+++ b/src/java/org/apache/fop/fo/InlineCharIterator.java
@@ -22,10 +22,13 @@ import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.util.CharUtilities;
 import java.util.NoSuchElementException;
 
-
+/**
+ * A recursive char iterator that indicates boundaries by returning
+ * an EOT char.
+ */
 public class InlineCharIterator extends RecursiveCharIterator {
-    private boolean bStartBoundary = false;
-    private boolean bEndBoundary = false;
+    private boolean startBoundary = false;
+    private boolean endBoundary = false;
 
     /**
      * @param fobj the object for whose character contents and for whose
@@ -39,20 +42,25 @@ public class InlineCharIterator extends RecursiveCharIterator {
 
 
     private void checkBoundaries(CommonBorderPaddingBackground bpb) {
-        bStartBoundary = (bpb.getBorderStartWidth(false) > 0
+        /* Current understanding is that an <fo:inline> is always a boundary for
+         * whitespace collapse if it has a border or not
+        startBoundary = (bpb.getBorderStartWidth(false) > 0
                        || bpb.getPaddingStart(false, null) > 0); // TODO do we need context here?
-        bEndBoundary = (bpb.getBorderEndWidth(false) > 0
+        endBoundary = (bpb.getBorderEndWidth(false) > 0
                      || bpb.getPaddingEnd(false, null) > 0); // TODO do we need context here?
+         */
+        startBoundary = true;
+        endBoundary = true;
     }
 
     /**
      * @return true if there are more characters
      */
     public boolean hasNext() {
-        if (bStartBoundary) {
+        if (startBoundary) {
             return true;
         }
-        return (super.hasNext() || bEndBoundary);
+        return (super.hasNext() || endBoundary);
         /* If super.hasNext() returns false,
          * we return true if we are going to return a "boundary" signal
          * else false.
@@ -64,8 +72,8 @@ public class InlineCharIterator extends RecursiveCharIterator {
      * @throws NoSuchElementException if there are no more characters
      */
     public char nextChar() throws NoSuchElementException {
-        if (bStartBoundary) {
-            bStartBoundary = false;
+        if (startBoundary) {
+            startBoundary = false;
             return CharUtilities.CODE_EOT;
         }
         try {
@@ -73,8 +81,8 @@ public class InlineCharIterator extends RecursiveCharIterator {
         } catch (NoSuchElementException e) {
             // Underlying has nothing more to return
             // Check end boundary char
-            if (bEndBoundary) {
-                bEndBoundary = false;
+            if (endBoundary) {
+                endBoundary = false;
                 return CharUtilities.CODE_EOT;
             } else {
                 throw e;
diff --git a/src/java/org/apache/fop/fo/flow/Block.java b/src/java/org/apache/fop/fo/flow/Block.java
index d6600f963..5e68a08f0 100644
--- a/src/java/org/apache/fop/fo/flow/Block.java
+++ b/src/java/org/apache/fop/fo/flow/Block.java
@@ -375,23 +375,41 @@ public class Block extends FObjMixed {
 
     private void handleWhiteSpace() {
         //getLogger().debug("fo:block: handleWhiteSpace");
-        if (firstInlineChild != null) {
-            boolean bInWS = false;
-            boolean bPrevWasLF = false;
-            
-            /* seenNonWSYet is an indicator used for trimming all leading 
-               whitespace for the first inline child of the block
-            */
-            boolean bSeenNonWSYet = false;
-            RecursiveCharIterator charIter =
-              new RecursiveCharIterator(this, firstInlineChild);
-            LFchecker lfCheck = new LFchecker(charIter);
-
-            while (charIter.hasNext()) {
-                char currentChar = charIter.nextChar();
-                switch (CharUtilities.classOf(currentChar)) {
-                    case CharUtilities.XMLWHITESPACE:
-                        /* Some kind of whitespace character, except linefeed. */
+        if (firstInlineChild == null) {
+            return; // Nothing to do
+        }
+        
+        boolean inWS = false; // True if we are in a run of white space
+        /*
+         * True if the last non white space char seen was a linefeed.
+         * We start from the beginning of a line so it defaults to True.
+         */
+        boolean prevWasLF = true; 
+
+        RecursiveCharIterator charIter =
+          new RecursiveCharIterator(this, firstInlineChild);
+        EOLchecker lfCheck = new EOLchecker(charIter);
+
+        while (charIter.hasNext()) {
+            char currentChar = charIter.nextChar();
+            int currentCharClass = CharUtilities.classOf(currentChar);
+            if (currentCharClass == CharUtilities.LINEFEED
+                && linefeedTreatment == EN_TREAT_AS_SPACE) {
+                // if we have a linefeed and it is suppose to be treated
+                // like a space, that's what we do and continue
+                currentChar = ' ';
+                charIter.replaceChar(' ');
+                currentCharClass = CharUtilities.classOf(currentChar);
+            }
+            switch (CharUtilities.classOf(currentChar)) {
+                case CharUtilities.XMLWHITESPACE:
+                    /* Some kind of whitespace character, except linefeed. */
+                    if (inWS && whiteSpaceCollapse == EN_TRUE) {
+                        // We are in a run of whitespace and should collapse
+                        // Just delete the char
+                        charIter.remove();
+                    } else {
+                        // Do the white space treatment here
                         boolean bIgnore = false;
 
                         switch (whiteSpaceTreatment) {
@@ -399,14 +417,16 @@ public class Block extends FObjMixed {
                                 bIgnore = true;
                                 break;
                             case Constants.EN_IGNORE_IF_BEFORE_LINEFEED:
-                                bIgnore = lfCheck.nextIsLF();
+                                bIgnore = linefeedTreatment == Constants.EN_PRESERVE
+                                            && lfCheck.nextIsLF();
                                 break;
                             case Constants.EN_IGNORE_IF_SURROUNDING_LINEFEED:
-                                bIgnore = (bPrevWasLF
-                                           || lfCheck.nextIsLF());
+                                bIgnore = (prevWasLF
+                                           || (linefeedTreatment == Constants.EN_PRESERVE
+                                               && lfCheck.nextIsLF()));
                                 break;
                             case Constants.EN_IGNORE_IF_AFTER_LINEFEED:
-                                bIgnore = bPrevWasLF;
+                                bIgnore = prevWasLF;
                                 break;
                             case Constants.EN_PRESERVE:
                                 // nothing to do now, replacement takes place later
@@ -415,111 +435,79 @@ public class Block extends FObjMixed {
                         // Handle ignore and replacement
                         if (bIgnore) {
                             charIter.remove();
-                        } else if (whiteSpaceCollapse == EN_TRUE) {
-                            if (bInWS || (linefeedTreatment == Constants.EN_PRESERVE
-                                        && (bPrevWasLF || lfCheck.nextIsLF()))) {
-                                charIter.remove();
-                            } else {
-                                // this is to retain a single space between words
-                                bInWS = true;
-                                // remove the space if no word in block 
-                                // encountered yet
-                                if (!bSeenNonWSYet) {
-                                    charIter.remove();
-                                } else {
-                                    if (currentChar != '\u0020') {
-                                        charIter.replaceChar('\u0020');
-                                    }
-                                }
-                            }
                         } else {
-                            // !bWScollapse
+                            // this is to retain a single space between words
+                            inWS = true;
                             if (currentChar != '\u0020') {
                                 charIter.replaceChar('\u0020');
                             }
                         }
-                        break;
-
-                    case CharUtilities.LINEFEED:
-                        /* A linefeed */
-                        lfCheck.reset();
-                        bPrevWasLF = true; // for following whitespace
+                    }
+                    break;
 
-                        switch (linefeedTreatment) {
-                            case Constants.EN_IGNORE:
-                                charIter.remove();
-                                break;
-                            case Constants.EN_TREAT_AS_SPACE:
-                                if (bInWS) {
-                                    // only if bWScollapse=true
-                                    charIter.remove();
-                                } else {
-                                    if (whiteSpaceCollapse == EN_TRUE) {
-                                        bInWS = true;
-                                        // remove the linefeed if no word in block 
-                                        // encountered yet
-                                        if (!bSeenNonWSYet) {
-                                            charIter.remove();
-                                        }
-                                    }
-                                    charIter.replaceChar('\u0020');
-                                }
-                                break;
-                            case Constants.EN_TREAT_AS_ZERO_WIDTH_SPACE:
-                                charIter.replaceChar('\u200b');
-                                // Fall through: this isn't XML whitespace
-                            case Constants.EN_PRESERVE:
-                                bInWS = false;
-                                break;
-                        }
-                        break;
-
-                    case CharUtilities.EOT:
-                        // A "boundary" objects such as non-character inline
-                        // or nested block object was encountered.
-                        // If any whitespace run in progress, finish it.
-                        // FALL THROUGH
-
-                    case CharUtilities.UCWHITESPACE: // Non XML-whitespace
-                    case CharUtilities.NONWHITESPACE:
-                        /* Any other character */
-                        bInWS = bPrevWasLF = false;
-                        bSeenNonWSYet = true;
-                        lfCheck.reset();
-                        break;
-                }
+                case CharUtilities.LINEFEED:
+                    /* A linefeed */
+                    switch (linefeedTreatment) {
+                        case Constants.EN_IGNORE:
+                            charIter.remove();
+                            break;
+                        case Constants.EN_TREAT_AS_ZERO_WIDTH_SPACE:
+                            charIter.replaceChar(CharUtilities.ZERO_WIDTH_SPACE);
+                            inWS = false;
+                            break;
+                        case Constants.EN_PRESERVE:
+                            lfCheck.reset();
+                            inWS = false;
+                            prevWasLF = true; // for following whitespace
+                            break;
+                    }
+                    break;
+
+                case CharUtilities.EOT:
+                    // A "boundary" objects such as non-character inline
+                    // or nested block object was encountered.
+                    // If any whitespace run in progress, finish it.
+                    // FALL THROUGH
+
+                default:
+                    /* Any other character */
+                    inWS = prevWasLF = false;
+                    lfCheck.reset();
+                    break;
             }
-            firstInlineChild = null;
         }
+        firstInlineChild = null;
     }
 
-    private static class LFchecker {
-        private boolean bNextIsLF = false;
+    private static class EOLchecker {
+        private boolean nextIsEOL = false;
         private RecursiveCharIterator charIter;
 
-        LFchecker(RecursiveCharIterator charIter) {
+        EOLchecker(RecursiveCharIterator charIter) {
             this.charIter = charIter;
         }
 
         boolean nextIsLF() {
-            if (bNextIsLF == false) {
+            if (nextIsEOL == false) {
                 CharIterator lfIter = charIter.mark();
                 while (lfIter.hasNext()) {
-                    char c = lfIter.nextChar();
-                    if (c == '\n') {
-                        bNextIsLF = true;
-                        break;
-                    } else if (CharUtilities.classOf(c)
-                            != CharUtilities.XMLWHITESPACE) {
-                        break;
+                    int charClass = CharUtilities.classOf(lfIter.nextChar());
+                    if (charClass == CharUtilities.LINEFEED) {
+                        nextIsEOL = true;
+                        return nextIsEOL;
+                    } else if (charClass != CharUtilities.XMLWHITESPACE) {
+                        return nextIsEOL;
                     }
                 }
+                // No more characters == end of block == end of line
+                nextIsEOL = true;
+                return nextIsEOL;
             }
-            return bNextIsLF;
+            return nextIsEOL;
         }
 
         void reset() {
-            bNextIsLF = false;
+            nextIsEOL = false;
         }
     }
      
diff --git a/src/java/org/apache/fop/fo/flow/Character.java b/src/java/org/apache/fop/fo/flow/Character.java
index 267bf47af..b5f47b02b 100644
--- a/src/java/org/apache/fop/fo/flow/Character.java
+++ b/src/java/org/apache/fop/fo/flow/Character.java
@@ -18,15 +18,12 @@
 
 package org.apache.fop.fo.flow;
 
-import org.xml.sax.Locator;
-
 import org.apache.fop.apps.FOPException;
 import org.apache.fop.datatypes.ColorType;
 import org.apache.fop.datatypes.Length;
 import org.apache.fop.fo.CharIterator;
 import org.apache.fop.fo.FONode;
 import org.apache.fop.fo.FObj;
-import org.apache.fop.fo.OneCharIterator;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
 import org.apache.fop.fo.properties.CommonAural;
@@ -39,6 +36,11 @@ import org.apache.fop.fo.properties.CommonTextDecoration;
 import org.apache.fop.fo.properties.KeepProperty;
 import org.apache.fop.fo.properties.Property;
 import org.apache.fop.fo.properties.SpaceProperty;
+import org.apache.fop.util.CharUtilities;
+
+import org.xml.sax.Locator;
+
+import java.util.NoSuchElementException;
 
 /**
  * This class represents the flow object 'fo:character'. Its use is defined by
@@ -156,8 +158,7 @@ public class Character extends FObj {
      * @see org.apache.fop.fo.FObj#charIterator
      */
     public CharIterator charIterator() {
-        return new OneCharIterator(character);
-        // But what if the character is ignored due to white space handling?
+        return new TextCharIterator();
     }
 
     /**
@@ -270,4 +271,31 @@ public class Character extends FObj {
         return FO_CHARACTER;
     }
     
+    private class TextCharIterator extends CharIterator {
+
+        private boolean bFirst = character != CharUtilities.CODE_EOT;
+        
+        public boolean hasNext() {
+            return bFirst;
+        }
+
+        public char nextChar() {
+            if (bFirst) {
+                bFirst = false;
+                return character;
+            } else {
+                throw new NoSuchElementException();
+            }
+        }
+
+        public void remove() {
+            character = CharUtilities.CODE_EOT;
+        }
+
+        public void replaceChar(char c) {
+            character = c;
+        }
+
+    }
+
 }
diff --git a/src/java/org/apache/fop/layoutmgr/LayoutManagerMapping.java b/src/java/org/apache/fop/layoutmgr/LayoutManagerMapping.java
index 84663dd84..2663d3359 100644
--- a/src/java/org/apache/fop/layoutmgr/LayoutManagerMapping.java
+++ b/src/java/org/apache/fop/layoutmgr/LayoutManagerMapping.java
@@ -78,6 +78,7 @@ import org.apache.fop.layoutmgr.inline.WrapperLayoutManager;
 import org.apache.fop.layoutmgr.list.ListBlockLayoutManager;
 import org.apache.fop.layoutmgr.list.ListItemLayoutManager;
 import org.apache.fop.layoutmgr.table.TableLayoutManager;
+import org.apache.fop.util.CharUtilities;
 
 /**
  * The default LayoutManager maker class
@@ -287,7 +288,10 @@ public class LayoutManagerMapping implements LayoutManagerMaker {
 
     public static class CharacterLayoutManagerMaker extends Maker {
         public void make(FONode node, List lms) {
-            lms.add(new CharacterLayoutManager((Character) node));
+            Character foCharacter = (Character) node;
+            if (foCharacter.getCharacter() != CharUtilities.CODE_EOT) {
+                lms.add(new CharacterLayoutManager(foCharacter));
+            }
         }
     }
 
diff --git a/src/java/org/apache/fop/layoutmgr/inline/AbstractGraphicsLayoutManager.java b/src/java/org/apache/fop/layoutmgr/inline/AbstractGraphicsLayoutManager.java
index 0f7b35aac..204c69e8f 100644
--- a/src/java/org/apache/fop/layoutmgr/inline/AbstractGraphicsLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/inline/AbstractGraphicsLayoutManager.java
@@ -117,7 +117,7 @@ public abstract class AbstractGraphicsLayoutManager extends LeafNodeLayoutManage
         if (len.getEnum() != EN_AUTO) {
             if (len.getEnum() == EN_SCALE_TO_FIT) {
                 if (bpd != -1) {
-                    cwidth = bpd;
+                    cheight = bpd;
                 }
             } else {
                 cheight = len.getValue(this);
diff --git a/src/java/org/apache/fop/render/xml/XMLRenderer.java b/src/java/org/apache/fop/render/xml/XMLRenderer.java
index da3260d54..ad6c9331a 100644
--- a/src/java/org/apache/fop/render/xml/XMLRenderer.java
+++ b/src/java/org/apache/fop/render/xml/XMLRenderer.java
@@ -200,7 +200,7 @@ public class XMLRenderer extends PrintRenderer {
             handleSAXException(saxe);
         }
     }
-    
+   
     /**
      * Adds a new attribute to the protected member variable "atts".
      * @param name name of the attribute