Fop
  1. Fop
  2. FOP-1120

[patch] force-page-count property implementation

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: None
    • Component/s: page-master/layout
    • Labels:
      None
    • Environment:
      Operating System: other
      Platform: Other
    • External issue ID:
      38087

      Description

      This patch (re)implements the force-page-count property.

      I tried to implement it with as little changes to the
      code as possible. Therefore some design issues and questions
      remain - see later.

      Description:
      The force-page-count property is handled by adding a method
      to the PageSequenceLayoutManger, because there are is a
      method that has all informations (static content, pagenumber,
      ...) about creating a new page (makeNewPage).
      The main problem was that the force-page-count property needs
      the initial-page-number of the next page-sequence which does
      not exist at this time.
      I solved this by preserving the current PageSequenceLayoutManger
      after the end of the page-sequence. I used the Root object
      for this because the last pagenumber of the sequence and other
      informations are also handed over this object from one
      page-sequence to the other. At the beginning of the following
      page-sequence the (stored) previous PageSequenceLayoutManger
      is called with the now current PageSequence object to finish
      this work.

      Changes to Root.java:

      • insert the variable previousPageSeqLM to store a
        PageSequenceLayoutManger for later use
        (and import the necessary class definition).

      Changes to PageSequenceLayoutManger.java:

      • split off the last part of activateLayout into a separate
        method (finishLeayout) because i need the same functionality
        after the force-page-count handling.
      • preserve the current PageSequenceLayoutManger object to the
        Root object at the end of the finishLayout method.
      • implement the force-page-count propterty in the
        checkForcePageCount method. This method needs the next
        PageSequence object as parameter. To get the initial-page-number
        property of this next page-sequence i copied the code from
        PageSequence.initPageNumber.

      Changes to PageSequence.java:

      • call the checkForcePageCount method of the preserved previous
        PageSequenceLayoutManger inside the startOfNode method
        immediately before initPageNumber so that this method is
        waware of possibly inserted pages of the previous page-sequence.
      • insert a getFocePageCount method to get the private variable.
      • insert a getInitialPageNumber method to get the private variable.

      Open issues / questions:

      • i think it very likely that the previousPageSeqLM variable of
        the Root class should be privat and there should be a
        setPreviousPageSeqLM and a getPreviousPageSeqLM method to
        hand the variable into and out of the object.
      • there was a second possiblity to implement this all
        by using the AreaTreeHandler:
      • instead of saving the PageSequenceLayoutManger object
        at the end of the finishLayout method of the
        PageSequenceLayoutManger it could be preserved at the
        end of the endPageSequence method of the AreaTreeHandler.
      • instead of calling the checkForcePageCount from the
        startOfNode method of PageSequence it could be called
        from the startPageSequence method of the AreaTreeHandler
        after receiving the rootFObj.
      • all other could be left nearly as with this patch with
        the exception that the startingPageNumber of the PageSequence
        has to be recalced after calling the checkForcePageCount
        because it is already calced at this point and does not
        know about possibly inserted pages from the
        checkForcePageCount method before.
        someone who has a more overall sight of the code has to
        decide where it fits best.

      I know the force-page-count property is of low priority, but
      it would be nice to get feedback, if someone can spare time
      to have a look at this patch.

      I hope i was not too verbose and that my patch is more
      usefull than the review is time-consuming and that i dont
      violate too much design guidelines.

      happy new year
      gerhard

      1. force-page-count.fo
        11 kB
        gerhard oettl
      2. force-page-count.fo
        10 kB
        gerhard oettl
      3. force-page-count.patch
        8 kB
        gerhard oettl
      4. force-page-count.patch
        7 kB
        gerhard oettl
      5. page-sequence_force-page-count_auto.xml
        5 kB
        gerhard oettl
      6. page-sequence_force-page-count_noauto.xml
        6 kB
        gerhard oettl

        Activity

        Hide
        gerhard oettl added a comment -

        Attachment force-page-count.patch has been added with description: the patch itself

        Show
        gerhard oettl added a comment - Attachment force-page-count.patch has been added with description: the patch itself
        Hide
        gerhard oettl added a comment -

        Attachment force-page-count.fo has been added with description: this fo i used for testing (no automated test case)

        Show
        gerhard oettl added a comment - Attachment force-page-count.fo has been added with description: this fo i used for testing (no automated test case)
        Hide
        Simon Pepping added a comment -

        Gerhard,

        Thanks for the patch. It looks fine.

        I am not quite happy with the fact that checkForcePageCount is called
        by the next page sequence. This is an interaction between page
        sequences, and it is better dealt with by the controlling structure,
        in this case AreaTreeHandler. In other words, I go with your
        suggestion in the second *.

        I also note that the object that is passed to checkForcePageCount can
        be narrowed down to Numeric nextPageSeq.getInitialPageNumber().

        There is a little error in the last page sequence but one in your demo
        file. It says that next is auto-even, which is not true.

        If you make the change to the patch, I will be happy to commit it.

        Simon

        Show
        Simon Pepping added a comment - Gerhard, Thanks for the patch. It looks fine. I am not quite happy with the fact that checkForcePageCount is called by the next page sequence. This is an interaction between page sequences, and it is better dealt with by the controlling structure, in this case AreaTreeHandler. In other words, I go with your suggestion in the second *. I also note that the object that is passed to checkForcePageCount can be narrowed down to Numeric nextPageSeq.getInitialPageNumber(). There is a little error in the last page sequence but one in your demo file. It says that next is auto-even, which is not true. If you make the change to the patch, I will be happy to commit it. Simon
        Hide
        gerhard oettl added a comment -

        ad the now 2c)
        At the time the checkForcePageCount is called from the
        AreaTreeHandler the initialPageNumber is already calced by the
        PageSequence without knowing about eventueally later added pages so
        it must be recalced. I did this by making the initPageNumber method
        public. If this should stay private i have to add a public
        recalcInitPageNumber method that does nothing than call the
        private initPageNumber method - at your decision.

        Show
        gerhard oettl added a comment - ad the now 2c) At the time the checkForcePageCount is called from the AreaTreeHandler the initialPageNumber is already calced by the PageSequence without knowing about eventueally later added pages so it must be recalced. I did this by making the initPageNumber method public. If this should stay private i have to add a public recalcInitPageNumber method that does nothing than call the private initPageNumber method - at your decision.
        Hide
        gerhard oettl added a comment -

        Attachment force-page-count.patch has been added with description: patch 2 (using area tree)

        Show
        gerhard oettl added a comment - Attachment force-page-count.patch has been added with description: patch 2 (using area tree)
        Hide
        gerhard oettl added a comment -

        Attachment force-page-count.fo has been added with description: a better fo example file

        Show
        gerhard oettl added a comment - Attachment force-page-count.fo has been added with description: a better fo example file
        Hide
        Simon Pepping added a comment -

        Applied. I rearranged the calls somewhat. This is the resulting call hierarchy:

        PageSequence.startOfNode
        AreaTreeHandler.startPageSequence
        PSLM.doForcePageCount (previous PS if not null)
        PSLM.finishPage
        PSLM.finishPageSequence (previous PS if not null)
        Root.notifyPageSequenceFinished
        AreaTreeHandler.notifyPageSequenceFinished
        PageSequence.initPageNumber

        PageSequence.endOfNode
        AreaTreeHandler.endPageSequence
        PSLM.activateLayout
        PSLM.finishPage

        Thanks for the patch.

        Simon

        Show
        Simon Pepping added a comment - Applied. I rearranged the calls somewhat. This is the resulting call hierarchy: PageSequence.startOfNode AreaTreeHandler.startPageSequence PSLM.doForcePageCount (previous PS if not null) PSLM.finishPage PSLM.finishPageSequence (previous PS if not null) Root.notifyPageSequenceFinished AreaTreeHandler.notifyPageSequenceFinished PageSequence.initPageNumber PageSequence.endOfNode AreaTreeHandler.endPageSequence PSLM.activateLayout PSLM.finishPage Thanks for the patch. Simon
        Hide
        gerhard oettl added a comment -

        Attachment page-sequence_force-page-count_auto.xml has been added with description: testcase for force-page-count="auto"

        Show
        gerhard oettl added a comment - Attachment page-sequence_force-page-count_auto.xml has been added with description: testcase for force-page-count="auto"
        Hide
        gerhard oettl added a comment -

        I started with pagenumber 201 to allow to concatenate to the testcase for
        force-page-count="auto"

        Show
        gerhard oettl added a comment - I started with pagenumber 201 to allow to concatenate to the testcase for force-page-count="auto"
        Hide
        gerhard oettl added a comment -

        Attachment page-sequence_force-page-count_noauto.xml has been added with description: testcase for all but auto

        Show
        gerhard oettl added a comment - Attachment page-sequence_force-page-count_noauto.xml has been added with description: testcase for all but auto
        Hide
        Simon Pepping added a comment -

        Thanks for the fine test cases. Re noauto: Do you really expect 18 pages? I get
        17, and I think that is correct.

        Show
        Simon Pepping added a comment - Thanks for the fine test cases. Re noauto: Do you really expect 18 pages? I get 17, and I think that is correct.
        Hide
        gerhard oettl added a comment -

        Yes i expect 17. I want to show the missing pageno 217 - a typical case of
        self-delusion.

        Show
        gerhard oettl added a comment - Yes i expect 17. I want to show the missing pageno 217 - a typical case of self-delusion.
        Hide
        Simon Pepping added a comment -

        Committed the test cases. Thanks. Simon

        Show
        Simon Pepping added a comment - Committed the test cases. Thanks. Simon
        Hide
        Glenn Adams added a comment -

        batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

        Show
        Glenn Adams added a comment - batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development