Bugzilla #53688: Wrong page number reported when a column overflows the region-body in a multi-column document

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

src/java/org/apache/fop/layoutmgr/PageBreaker.java

         return new PageBreakingLayoutListener() {
 
             public void notifyOverflow(int part, int amount, FObj obj) {
-                Page p = pageProvider.getPage(
-                            false, part, PageProvider.RELTO_CURRENT_ELEMENT_LIST);
+                Page p = pageProvider.getPageFromColumnIndex(part);
                 RegionBody body = (RegionBody)p.getSimplePageMaster().getRegion(
                         Region.FO_REGION_BODY);
                 BlockLevelEventProducer eventProducer = BlockLevelEventProducer.Provider.get(

src/java/org/apache/fop/layoutmgr/PageProvider.java

         return this.lastReportedBPD;
     }
 
-    // Wish there were a more elegant way to do this in Java
-    private int[] getColIndexAndColCount(int index) {
+    private static class Column {
+
+        final Page page;
+
+        final int pageIndex;
+
+        final int colIndex;
+
+        final int columnCount;
+
+        Column(Page page, int pageIndex, int colIndex, int columnCount) {
+            this.page = page;
+            this.pageIndex = pageIndex;
+            this.colIndex = colIndex;
+            this.columnCount = columnCount;
+        }
+
+    }
+
+    private Column getColumn(int index) {
         int columnCount = 0;
         int colIndex = startColumnOfCurrentElementList + index;
         int pageIndex = -1;
+        Page page;
         do {
             colIndex -= columnCount;
             pageIndex++;
-            Page page = getPage(false, pageIndex, RELTO_CURRENT_ELEMENT_LIST);
+            page = getPage(false, pageIndex, RELTO_CURRENT_ELEMENT_LIST);
             columnCount = page.getPageViewport().getCurrentSpan().getColumnCount();
         } while (colIndex >= columnCount);
-        return new int[] {colIndex, columnCount};
+        return new Column(page, pageIndex, colIndex, columnCount);
     }
 
     /**
      * than, equal to or greater than the IPD of the following part
      */
     public int compareIPDs(int index) {
-        int columnCount = 0;
-        int colIndex = startColumnOfCurrentElementList + index;
-        int pageIndex = -1;
-        Page page;
-        do {
-            colIndex -= columnCount;
-            pageIndex++;
-            page = getPage(false, pageIndex, RELTO_CURRENT_ELEMENT_LIST);
-            columnCount = page.getPageViewport().getCurrentSpan().getColumnCount();
-        } while (colIndex >= columnCount);
-        if (colIndex + 1 < columnCount) {
+        Column column = getColumn(index);
+        if (column.colIndex + 1 < column.columnCount) {
             // Next part is a column on same page => same IPD
             return 0;
         } else {
-            Page nextPage = getPage(false, pageIndex + 1, RELTO_CURRENT_ELEMENT_LIST);
-            return page.getPageViewport().getBodyRegion().getIPD()
+            Page nextPage = getPage(false, column.pageIndex + 1, RELTO_CURRENT_ELEMENT_LIST);
+            return column.page.getPageViewport().getBodyRegion().getIPD()
                     - nextPage.getPageViewport().getBodyRegion().getIPD();
         }
     }
      * @return  {@code true} if the break starts a new page
      */
     boolean startPage(int index) {
-        return getColIndexAndColCount(index)[0] == 0;
+        return getColumn(index).colIndex == 0;
     }
 
     /**
      * @return  {@code true} if the break ends a page
      */
     boolean endPage(int index) {
-        int[] colIndexAndColCount = getColIndexAndColCount(index);
-        return colIndexAndColCount[0] == colIndexAndColCount[1] - 1;
+        Column column = getColumn(index);
+        return column.colIndex == column.columnCount - 1;
     }
 
     /**
      * @return  the number of columns
      */
     int getColumnCount(int index) {
-        return getColIndexAndColCount(index)[1];
+        return getColumn(index).columnCount;
     }
 
     /**
      * @return the requested part index
      */
     public int getStartingPartIndexForLastPage(int partCount) {
-        int result = 0;
-        int idx = 0;
-        int pageIndex = 0;
-        int colIndex = startColumnOfCurrentElementList;
-        Page page = getPage(
-                false, pageIndex, RELTO_CURRENT_ELEMENT_LIST);
-        while (idx < partCount) {
-            if ((colIndex >= page.getPageViewport().getCurrentSpan().getColumnCount())) {
-                colIndex = 0;
-                pageIndex++;
-                page = getPage(
-                        false, pageIndex, RELTO_CURRENT_ELEMENT_LIST);
-                result = idx;
-            }
-            colIndex++;
-            idx++;
-        }
-        return result;
+        int lastPartIndex = partCount - 1;
+        return lastPartIndex - getColumn(lastPartIndex).colIndex;
+    }
+
+    Page getPageFromColumnIndex(int columnIndex) {
+        return getColumn(columnIndex).page;
     }
 
     /**
                 log.trace("last page requested: " + index);
             }
         }
-        while (intIndex >= cachedPages.size()) {
+        if (intIndex > cachedPages.size()) {
+            throw new UnsupportedOperationException("Cannot handle holes in page cache");
+        } else if (intIndex == cachedPages.size()) {
             if (log.isTraceEnabled()) {
                 log.trace("Caching " + index);
             }

status.xml

       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
     <release version="FOP Trunk" date="TBD">
+      <action context="Layout" dev="VH" type="fix" fixes-bug="53688">
+        Wrong page number reported when a column overflows the region-body in a multi-column 
+        document.
+      </action>
       <action context="Renderers" dev="VH" type="add" fixes-bug="53639">
         When PDF accessibility is enabled, the Scope attribute must be present in the structure tree 
         for table header elements.

test/events/region-body_overflow.fo

+<?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$ -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+  <fo:layout-master-set>
+    <fo:simple-page-master master-name="page"
+      page-height="80pt" page-width="530pt" margin="10pt" margin-bottom="0">
+      <fo:region-body margin-bottom="20pt" column-count="5" column-gap="10pt"/>
+      <fo:region-after extent="15pt"/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="page" font-size="8pt" line-height="10pt">
+    <fo:static-content flow-name="xsl-region-after">
+      <fo:block text-align="center"><fo:page-number/></fo:block>
+    </fo:static-content>
+    <fo:flow flow-name="xsl-region-body">
+
+      <fo:block>Page 1 Column 1 Line 1</fo:block>
+      <fo:block>Page 1 Column 1 Line 2</fo:block>
+      <fo:block>Page 1 Column 1 Line 3</fo:block>
+      <fo:block>Page 1 Column 1 Line 4</fo:block>
+      <fo:block>Page 1 Column 1 Line 5</fo:block>
+      <fo:block>Page 1 Column 2 Line 1</fo:block>
+      <fo:block>Page 1 Column 2 Line 2</fo:block>
+      <fo:block>Page 1 Column 2 Line 3</fo:block>
+      <fo:block>Page 1 Column 2 Line 4</fo:block>
+      <fo:block>Page 1 Column 2 Line 5</fo:block>
+      <fo:block>Page 1 Column 3 Line 1</fo:block>
+      <fo:block>Page 1 Column 3 Line 2</fo:block>
+      <fo:block>Page 1 Column 3 Line 3</fo:block>
+      <fo:block>Page 1 Column 3 Line 4</fo:block>
+      <fo:block>Page 1 Column 3 Line 5</fo:block>
+      <fo:block>Page 1 Column 4 Line 1</fo:block>
+      <fo:block>Page 1 Column 4 Line 2</fo:block>
+      <fo:block>Page 1 Column 4 Line 3</fo:block>
+      <fo:block>Page 1 Column 4 Line 4</fo:block>
+      <fo:block>Page 1 Column 4 Line 5</fo:block>
+      <fo:block-container height="55pt" background-color="#F0F0F0">
+        <fo:block>Page 1 Column 5 Line 1</fo:block>
+        <fo:block>Page 1 Column 5 Line 2</fo:block>
+        <fo:block>Page 1 Column 5 Line 3</fo:block>
+        <fo:block>Page 1 Column 5 Line 4</fo:block>
+        <fo:block>Page 1 Column 5 Line 5</fo:block>
+      </fo:block-container>
+
+      <fo:block break-before="page">Page 2 Column 1 Line 1</fo:block>
+      <fo:block>Page 2 Column 1 Line 2</fo:block>
+      <fo:block>Page 2 Column 1 Line 3</fo:block>
+      <fo:block>Page 2 Column 1 Line 4</fo:block>
+      <fo:block>Page 2 Column 1 Line 5</fo:block>
+      <fo:block>Page 2 Column 2 Line 1</fo:block>
+      <fo:block>Page 2 Column 2 Line 2</fo:block>
+      <fo:block>Page 2 Column 2 Line 3</fo:block>
+      <fo:block>Page 2 Column 2 Line 4</fo:block>
+      <fo:block>Page 2 Column 2 Line 5</fo:block>
+      <fo:block>Page 2 Column 3 Line 1</fo:block>
+      <fo:block>Page 2 Column 3 Line 2</fo:block>
+      <fo:block>Page 2 Column 3 Line 3</fo:block>
+      <fo:block>Page 2 Column 3 Line 4</fo:block>
+      <fo:block>Page 2 Column 3 Line 5</fo:block>
+      <fo:block>Page 2 Column 4 Line 1</fo:block>
+      <fo:block>Page 2 Column 4 Line 2</fo:block>
+      <fo:block>Page 2 Column 4 Line 3</fo:block>
+      <fo:block>Page 2 Column 4 Line 4</fo:block>
+      <fo:block>Page 2 Column 4 Line 5</fo:block>
+      <fo:block>Page 2 Column 5 Line 1</fo:block>
+      <fo:block>Page 2 Column 5 Line 2</fo:block>
+      <fo:block>Page 2 Column 5 Line 3</fo:block>
+      <fo:block>Page 2 Column 5 Line 4</fo:block>
+      <fo:block>Page 2 Column 5 Line 5</fo:block>
+
+      <fo:block>Page 3 Column 1 Line 1</fo:block>
+      <fo:block>Page 3 Column 1 Line 2</fo:block>
+      <fo:block>Page 3 Column 1 Line 3</fo:block>
+      <fo:block>Page 3 Column 1 Line 4</fo:block>
+      <fo:block>Page 3 Column 1 Line 5</fo:block>
+      <fo:block>Page 3 Column 2 Line 1</fo:block>
+      <fo:block>Page 3 Column 2 Line 2</fo:block>
+      <fo:block>Page 3 Column 2 Line 3</fo:block>
+      <fo:block>Page 3 Column 2 Line 4</fo:block>
+      <fo:block>Page 3 Column 2 Line 5</fo:block>
+      <fo:block>Page 3 Column 3 Line 1</fo:block>
+      <fo:block>Page 3 Column 3 Line 2</fo:block>
+      <fo:block>Page 3 Column 3 Line 3</fo:block>
+      <fo:block>Page 3 Column 3 Line 4</fo:block>
+      <fo:block>Page 3 Column 3 Line 5</fo:block>
+      <fo:block>Page 3 Column 4 Line 1</fo:block>
+      <fo:block>Page 3 Column 4 Line 2</fo:block>
+      <fo:block>Page 3 Column 4 Line 3</fo:block>
+      <fo:block>Page 3 Column 4 Line 4</fo:block>
+      <fo:block>Page 3 Column 4 Line 5</fo:block>
+      <fo:block>Page 3 Column 5 Line 1</fo:block>
+      <fo:block>Page 3 Column 5 Line 2</fo:block>
+      <fo:block>Page 3 Column 5 Line 3</fo:block>
+      <fo:block>Page 3 Column 5 Line 4</fo:block>
+      <fo:block>Page 3 Column 5 Line 5</fo:block>
+
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>

test/java/org/apache/fop/events/EventChecker.java

 
 package org.apache.fop.events;
 
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 /**
 
     private final String expectedEventID;
 
+    private final Map<String, Object> expectedParams;
+
     private boolean eventReceived;
 
-    EventChecker(String expectedEventID) {
+    EventChecker(String expectedEventID, Map<String, Object> expectedParams) {
         this.expectedEventID = expectedEventID;
+        this.expectedParams = expectedParams;
     }
 
     public void processEvent(Event event) {
         String id = event.getEventID();
         if (id.equals(expectedEventID)) {
             eventReceived = true;
+            for (Map.Entry<String, Object> param : expectedParams.entrySet()) {
+                assertEquals(event.getParam(param.getKey()), param.getValue());
+            }
         }
     }
 

test/java/org/apache/fop/events/EventProcessingTestCase.java

 import java.io.File;
 import java.io.InputStream;
 import java.net.URI;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
         CONFIG_BASE_DIR = base.resolve("test/config/");
 
     }
-    public void doTest(InputStream inStream, URI fopConf, String expectedEventID, String mimeType)
-            throws Exception {
-        EventChecker eventChecker = new EventChecker(expectedEventID);
+
+    public void doTest(InputStream inStream, URI fopConf, String expectedEventID, String mimeType,
+            Map<String, Object> expectedParams) throws Exception {
+        EventChecker eventChecker = new EventChecker(expectedEventID, expectedParams);
         FopFactory fopFactory;
         if (fopConf != null) {
             fopFactory = FopFactory.newInstance(new File(fopConf));
         Result res = new SAXResult(fop.getDefaultHandler());
         transformer.transform(src, res);
         eventChecker.end();
+
+    }
+
+    public void doTest(InputStream inStream, URI fopConf, String expectedEventID, String mimeType)
+            throws Exception {
+        Map<String, Object> noParams = Collections.emptyMap();
+        doTest(inStream, fopConf, expectedEventID, mimeType, noParams);
+    }
+
+    public void doTest(String filename, String expectedEventID, Map<String, Object> expectedParams)
+            throws Exception {
+        doTest(BASE_DIR.resolve(filename).toURL().openStream(), null, expectedEventID,
+                MimeConstants.MIME_PDF, expectedParams);
     }
 
     public void doTest(String filename, String expectedEventID) throws Exception {
     public void testViewportBPDOverflow() throws Exception {
         doTest("viewport-overflow.fo", BlockLevelEventProducer.class.getName() + ".viewportBPDOverflow");
     }
+
+    @Test
+    public void testPageOverflow() throws Exception {
+        Map<String, Object> params = new HashMap<String, Object>();
+        params.put("page", "1");
+        doTest("region-body_overflow.fo", BlockLevelEventProducer.class.getName() + ".regionOverflow",
+                params);
+    }
 }