FOP
  1. FOP
  2. FOP-1911

Incorrect refactoring in src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java (r1069154)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: None
    • Component/s: layout/unqualified
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: PC
    • External issue ID:
      50965

      Description

      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);

        Activity

        Hide
        Andreas L. Delmelle added a comment -

        (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!

        Show
        Andreas L. Delmelle added a comment - (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!
        Hide
        Martin K added a comment -

        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>

        Show
        Martin K added a comment - 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>
        Hide
        Andreas L. Delmelle added a comment -

        Fixed in r1085820 (see: http://svn.apache.org/viewvc?rev=1085820&view=rev)

        Thanks for reporting!

        Show
        Andreas L. Delmelle added a comment - Fixed in r1085820 (see: http://svn.apache.org/viewvc?rev=1085820&view=rev ) Thanks for reporting!
        Hide
        Glenn Adams added a comment -

        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

        Show
        Glenn Adams added a comment - 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

          People

          • Assignee:
            fop-dev
            Reporter:
            Martin K
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development