Bug 50965 - Incorrect refactoring in src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java (r1069154)
Summary: Incorrect refactoring in src/java/org/apache/fop/layoutmgr/BlockContainerLayo...
Status: CLOSED FIXED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: page-master/layout (show other bugs)
Version: all
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-24 07:21 UTC by Martin K
Modified: 2012-04-01 06:18 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin K 2011-03-24 07:21:31 UTC
r1069154 refactored some code to the function BlockContainerLayoutManager.setupAreaDimensions()

contentRectOffsetX and contentRectOffsetY are now only initialised to 0, when the class is instantiated. The previous version did this for every execution of this code.
As setupAreaDimensions() can be executed multiple times, this yields to layout errors.

--- a/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java
@@ -346,6 +346,8 @@ public class BlockContainerLayoutManager extends BlockStackingLayoutManager
             updateContentAreaIPDwithOverconstrainedAdjust(contentWidth);
         }

+        contentRectOffsetX = 0;
+        contentRectOffsetY = 0;
         contentRectOffsetX += fo.getCommonMarginBlock().startIndent.getValue(this);
         contentRectOffsetY += fo.getCommonBorderPaddingBackground().getBorderBeforeWidth(false);
         contentRectOffsetY += fo.getCommonBorderPaddingBackground().getPaddingBefore(false, this);
Comment 1 Andreas L. Delmelle 2011-03-24 13:53:48 UTC
(In reply to comment #0)
> r1069154 refactored some code to the function
> BlockContainerLayoutManager.setupAreaDimensions()
> 
> contentRectOffsetX and contentRectOffsetY are now only initialised to 0, when
> the class is instantiated. The previous version did this for every execution of
> this code.

Apparently a scenario for which we do not yet have a regression test. Since you discovered it, can you please attach a small FO that reproduces the issue so we can add that to our test suite?

Thanks!
Comment 2 Martin K 2011-03-24 18:00:33 UTC
Please compare the area tree before/after this change:

<?xml version="1.0" encoding="UTF-8"?>
<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
  <fo:layout-master-set>
    <fo:simple-page-master master-name="page" page-height="100mm" page-width="100mm">
      <fo:region-body/>
    </fo:simple-page-master>
    <fo:page-sequence-master master-name="ps">
      <fo:repeatable-page-master-alternatives>
        <fo:conditional-page-master-reference master-reference="page" page-position="first"/>
        <fo:conditional-page-master-reference master-reference="page" page-position="rest"/>
        <fo:conditional-page-master-reference master-reference="page" page-position="last"/>
      </fo:repeatable-page-master-alternatives>
    </fo:page-sequence-master>
  </fo:layout-master-set>
  <fo:page-sequence master-reference="ps">
    <fo:flow flow-name="xsl-region-body">
      <fo:block-container margin-left="20mm">
        <fo:block-container page-break-before="always">
          <fo:block>A</fo:block>
        </fo:block-container>
      </fo:block-container>
    </fo:flow>
  </fo:page-sequence>
</fo:root>
Comment 3 Andreas L. Delmelle 2011-03-26 18:44:28 UTC
Fixed in r1085820 (see: http://svn.apache.org/viewvc?rev=1085820&view=rev)

Thanks for reporting!
Comment 4 Glenn Adams 2012-04-01 06:18:28 UTC
batch transition to closed; if someone wishes to restore one of these to resolved in order to perform a verification step, then feel free to do so