FOP 2388: Fix Arabic text left justified on the first cell of a table with writing mode rl-tb.

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

src/java/org/apache/fop/complexscripts/bidi/BidiResolver.java

@@ -57,7 +57,17 @@ public final class BidiResolver {
         if (log.isDebugEnabled()) {
             log.debug("BD: RESOLVE: " + ps);
         }
-        List ranges = pruneEmptyRanges(ps.collectDelimitedTextRanges(new Stack()));
+        // 1. collect delimited text ranges
+        List ranges = ps.collectDelimitedTextRanges(new Stack());
+        if (log.isDebugEnabled()) {
+            dumpRanges("BD: RESOLVE: RANGES:", ranges);
+        }
+        // 2. prune empty ranges
+        ranges = pruneEmptyRanges(ranges);
+        if (log.isDebugEnabled()) {
+            dumpRanges("BD: RESOLVE: PRUNED RANGES:", ranges);
+        }
+        // 3. resolve inline directionaly of unpruned ranges
         resolveInlineDirectionality(ranges);
     }
 
@@ -223,8 +233,14 @@ public final class BidiResolver {
             log.debug(ir);
         }
     }
-
-    private static List pruneEmptyRanges(Stack ranges) {
+    private static void dumpRanges(String header, List ranges) {
+        log.debug(header);
+        for (Iterator it = ranges.iterator(); it.hasNext(); ) {
+            DelimitedTextRange r = (DelimitedTextRange) it.next();
+            log.debug(r);
+        }
+    }
+    private static List pruneEmptyRanges(List ranges) {
         Vector rv = new Vector();
         for (Iterator it = ranges.iterator(); it.hasNext(); ) {
             DelimitedTextRange r = (DelimitedTextRange) it.next();

src/java/org/apache/fop/complexscripts/bidi/UnicodeBidiAlgorithm.java

@@ -38,7 +38,7 @@ public final class UnicodeBidiAlgorithm implements BidiConstants {
     /**
      * logging instance
      */
-    private static final Log log = LogFactory.getLog(BidiResolver.class);
+    private static final Log log = LogFactory.getLog(UnicodeBidiAlgorithm.class);
 
     private UnicodeBidiAlgorithm() {
     }

src/java/org/apache/fop/fo/FONode.java

@@ -936,7 +936,7 @@ public abstract class FONode implements Cloneable {
      * @param ranges a stack of delimited text ranges
      * @return the (possibly) updated stack of delimited text ranges
      */
-    public Stack collectDelimitedTextRanges(Stack ranges) {
+    public Stack collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges) {
         // if boundary before, then push new range
         if (isRangeBoundaryBefore()) {
             maybeNewRange(ranges);
@@ -944,7 +944,7 @@ public abstract class FONode implements Cloneable {
         // get current range, if one exists
         DelimitedTextRange currentRange;
         if (ranges.size() > 0) {
-            currentRange = (DelimitedTextRange) ranges.peek();
+            currentRange = ranges.peek();
         } else {
             currentRange = null;
         }
@@ -965,7 +965,7 @@ public abstract class FONode implements Cloneable {
      * @param currentRange the current range or null (if none)
      * @return the (possibly) updated stack of delimited text ranges
      */
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges, DelimitedTextRange currentRange) {
         for (Iterator it = getChildNodes(); (it != null) && it.hasNext();) {
             ranges = ((FONode) it.next()).collectDelimitedTextRanges(ranges);
         }
@@ -981,31 +981,18 @@ public abstract class FONode implements Cloneable {
     }
 
     /**
-     * Conditionally add a new delimited text range to RANGES, where new range is
-     * associated with current FONode. A new text range is added unless all of the following are
-     * true:
-     * <ul>
-     * <li>there exists a current range RCUR in RANGES</li>
-     * <li>RCUR is empty</li>
-     * <li>the node of the RCUR is the same node as FN or a descendent node of FN</li>
-     * </ul>
+     * Conditionally add a new delimited text range to RANGES. Always add new
+     * range unless there are no ranges on the stack yet and this node is not a block item.
      * @param ranges stack of delimited text ranges
      * @return new range (if constructed and pushed onto stack) or current range (if any) or null
      */
-    private DelimitedTextRange maybeNewRange(Stack ranges) {
-        DelimitedTextRange rCur = null; // current range (top of range stack)
-        DelimitedTextRange rNew = null; // new range to be pushed onto range stack
-        if (ranges.empty()) {
-            if (isBidiRangeBlockItem()) {
-                rNew = new DelimitedTextRange(this);
-            }
+    private DelimitedTextRange maybeNewRange(Stack<DelimitedTextRange> ranges) {
+        DelimitedTextRange rCur = !ranges.empty() ? ranges.peek() : null;
+        DelimitedTextRange rNew;
+        if ((rCur != null) || isBidiRangeBlockItem()) {
+            rNew = new DelimitedTextRange(this);
         } else {
-            rCur = (DelimitedTextRange) ranges.peek();
-            if (rCur != null) {
-                if (!rCur.isEmpty() || !isSelfOrDescendent(rCur.getNode(), this)) {
-                    rNew = new DelimitedTextRange(this);
-                }
-            }
+            rNew = null;
         }
         if (rNew != null) {
             ranges.push(rNew);
@@ -1023,18 +1010,6 @@ public abstract class FONode implements Cloneable {
         return isDelimitedTextRangeBoundary(Constants.EN_AFTER);
     }
 
-    /**
-     * Determine if node N2 is the same or a descendent of node N1.
-     */
-    private static boolean isSelfOrDescendent(FONode n1, FONode n2) {
-        for (FONode n = n2; n != null; n = n.getParent()) {
-            if (n == n1) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     /**
      * Base iterator interface over a FO's children
      */

src/java/org/apache/fop/fo/FOText.java

@@ -802,7 +802,8 @@ public class FOText extends FONode implements CharSequence, TextFragment {
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         if (currentRange != null) {
             currentRange.append(charIterator(), this);
         }

src/java/org/apache/fop/fo/flow/AbstractGraphics.java

@@ -259,7 +259,8 @@ public abstract class AbstractGraphics extends FObj
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         if (currentRange != null) {
             currentRange.append(CharUtilities.OBJECT_REPLACEMENT_CHARACTER, this);
         }

src/java/org/apache/fop/fo/flow/AbstractPageNumberCitation.java

@@ -206,7 +206,8 @@ public abstract class AbstractPageNumberCitation extends FObj
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         if (currentRange != null) {
             currentRange.append(CharUtilities.OBJECT_REPLACEMENT_CHARACTER, this);
         }

src/java/org/apache/fop/fo/flow/BidiOverride.java

@@ -97,7 +97,8 @@ public class BidiOverride extends Inline {
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         char pfx = 0;
         char sfx = 0;
         int unicodeBidi = getUnicodeBidi();

src/java/org/apache/fop/fo/flow/Character.java

@@ -241,7 +241,8 @@ public class Character extends FObj implements StructureTreeElementHolder {
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         if (currentRange != null) {
             currentRange.append(charIterator(), this);
         }

src/java/org/apache/fop/fo/flow/Leader.java

@@ -203,7 +203,8 @@ public class Leader extends InlineLevel {
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         if (currentRange != null) {
             if (leaderPattern == EN_USECONTENT) {
                 ranges = super.collectDelimitedTextRanges(ranges, currentRange);

src/java/org/apache/fop/fo/flow/ListItem.java

@@ -206,7 +206,8 @@ public class ListItem extends FObj implements BreakPropertySet, CommonAccessibil
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         ListItemLabel label = getLabel();
         if (label != null) {
             ranges = label.collectDelimitedTextRanges(ranges);

src/java/org/apache/fop/fo/flow/table/Table.java

@@ -608,7 +608,8 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder, Break
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         // header sub-tree
         TableHeader header = getTableHeader();
         if (header != null) {

src/java/org/apache/fop/fo/pagination/PageSequence.java

@@ -416,7 +416,8 @@ public class PageSequence extends AbstractPageSequence implements WritingModeTra
     }
 
     @Override
-    protected Stack collectDelimitedTextRanges(Stack ranges, DelimitedTextRange currentRange) {
+    protected Stack<DelimitedTextRange> collectDelimitedTextRanges(Stack<DelimitedTextRange> ranges,
+        DelimitedTextRange currentRange) {
         // collect ranges from static content flows
         Map<String, FONode> flows = getFlowMap();
         if (flows != null) {

status.xml

@@ -58,7 +58,10 @@
       users, i.e. when the behaviour changes and could affect the layout of existing
       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
-    <release version="FOP Trunk" date="01 November 2013">
+    <release version="FOP Trunk" date="06 July 2014">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="FOP-2388">
+          Fix Arabic text left justified on the first cell of a table with writing mode rl-tb.
+      </action>
       <action context="Code" dev="GA" type="fix" fixes-bug="FOP-2311">
           Eliminate regression to shorthand properties from Bugzilla 37114 (FOP-1069).
       </action>

test/layoutengine/standard-testcases/table_jira2388.xml

@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+<!-- $Id$ -->
+<testcase>
+  <info>
+    <p>
+      This is a testcase for jira #2388: when a 2-row table with WM(rl-tb) is contained in a block
+      with WM(lr-tb), then content from both rows should, by default, use WM(rl-tb) from table. Prior
+      to fix, the first row was obtaining default WM(lr-tb) from the container block rather than table.
+    </p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master margin="20mm" master-name="letterPage" page-width="210mm" page-height="297mm">
+          <fo:region-body region-name="letterPageBody"/>
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="letterPage">
+        <fo:flow flow-name="letterPageBody">
+          <fo:block>
+            <fo:table writing-mode="rl-tb" table-layout="fixed" width="100%">
+              <fo:table-column column-width="proportional-column-width(100)" column-number="1"/>
+              <fo:table-body>
+                <fo:table-row>
+                  <fo:table-cell>
+                    <fo:block font-family="Arial" font-size="12pt" font-weight="normal">يرحب بكم ستاندرد تشارترد بنك و يتطلع لتقديم خدمات الحسابات المصرفية لكم</fo:block>
+                  </fo:table-cell>
+                </fo:table-row>
+                <fo:table-row>
+                  <fo:table-cell>
+                    <fo:block font-family="Arial" font-size="12pt" font-weight="normal">يرحب بكم ستاندرد تشارترد بنك و يتطلع لتقديم خدمات الحسابات المصرفية لكم</fo:block>
+                  </fo:table-cell>
+                </fo:table-row>
+              </fo:table-body>
+            </fo:table>
+          </fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="2" xpath="count(//lineArea)"/>
+    <eval expected="1" xpath="(//lineArea)[1]/@level"/>
+    <eval expected="88800" xpath="(//lineArea)[1]/@end-indent"/>
+    <eval expected="1" xpath="(//lineArea)[2]/@level"/>
+    <eval expected="88800" xpath="(//lineArea)[2]/@end-indent"/>
+  </checks>
+</testcase>