Fop
  1. Fop
  2. FOP-1520

page-number-citation shows no value or incorrect value.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: None
    • Component/s: pdf
    • Labels:
      None
    • Environment:
      Operating System: Windows 2000
      Platform: PC
    • External issue ID:
      44794

      Description

      page-number-citation-last does not work with fo:inline (page 1 and 3, no value)
      page-number-citation works partially with fo:inline (page 1, no value)
      page-number-citation-last works partially with fo:block (page 1, wrong value)

        Activity

        Hide
        Guillaume Levrero added a comment -

        Attachment FOP_BUG_page-number-citation.fo has been added with description: FO to reproduce bug.

        Show
        Guillaume Levrero added a comment - Attachment FOP_BUG_page-number-citation.fo has been added with description: FO to reproduce bug.
        Hide
        Andreas L. Delmelle added a comment -

        Apologies for the delay.

        Note to self / other devs:
        Part of the issue was already known:
        ID resolution does not take into account whether the LM that added the area is already finished, for page-number-citation-last.

        For the failing page-number-citation on fo:inline, page 1:
        IIC, id resolution also does not account for inline areas that will be added to the page later. The citation itself generates an unresolved area, but the actual id gets added later and marked resolved, before the page is finished.

        Show
        Andreas L. Delmelle added a comment - Apologies for the delay. Note to self / other devs: Part of the issue was already known: ID resolution does not take into account whether the LM that added the area is already finished, for page-number-citation-last. For the failing page-number-citation on fo:inline, page 1: IIC, id resolution also does not account for inline areas that will be added to the page later. The citation itself generates an unresolved area, but the actual id gets added later and marked resolved, before the page is finished.
        Hide
        Andreas L. Delmelle added a comment -

        Note: just discovered that there is currently also a slight inefficiency in the way inline ids are added to the page. addId() gets called for every line spanned by the inline.
        Will need to look into how we can avoid that, since the page viewport list in idTracker fills up with duplicates, and only the first one gets removed upon resolution...

        I've just committed a temporary hack to at least avoid the creation of duplicates in the List.

        Show
        Andreas L. Delmelle added a comment - Note: just discovered that there is currently also a slight inefficiency in the way inline ids are added to the page. addId() gets called for every line spanned by the inline. Will need to look into how we can avoid that, since the page viewport list in idTracker fills up with duplicates, and only the first one gets removed upon resolution... I've just committed a temporary hack to at least avoid the creation of duplicates in the List.
        Hide
        Andreas L. Delmelle added a comment -

        Further notes:
        In the related code, there's some preparations for resolving the page-number-citation-last correctly, but the problem is that this depends on calls to notifyEndOfLayout().

        Now, checking for blocks, that call gets made with every addAreas() call. Still have to figure out whether this was intended or necessary for some reason, but currently this means that the IDTracker mistakenly gets the impression that the LMs are finished on every page they add areas to.

        For inlines, this method is not used yet, but if we would go about it the same way as for blocks, notifyEndOfLayout() would be called for each line.

        The isFinished() method also cannot be used, since for nearly all LMs this is used to indicate whether they are finished 'generating the element list for their content'.

        Show
        Andreas L. Delmelle added a comment - Further notes: In the related code, there's some preparations for resolving the page-number-citation-last correctly, but the problem is that this depends on calls to notifyEndOfLayout(). Now, checking for blocks, that call gets made with every addAreas() call. Still have to figure out whether this was intended or necessary for some reason, but currently this means that the IDTracker mistakenly gets the impression that the LMs are finished on every page they add areas to. For inlines, this method is not used yet, but if we would go about it the same way as for blocks, notifyEndOfLayout() would be called for each line. The isFinished() method also cannot be used, since for nearly all LMs this is used to indicate whether they are finished 'generating the element list for their content'.
        Hide
        Jeremias Maerki added a comment -

        (In reply to comment #3)
        > Further notes:
        > In the related code, there's some preparations for resolving the
        > page-number-citation-last correctly, but the problem is that this depends on
        > calls to notifyEndOfLayout().
        >
        > Now, checking for blocks, that call gets made with every addAreas() call. Still
        > have to figure out whether this was intended or necessary for some reason, but
        > currently this means that the IDTracker mistakenly gets the impression that the
        > LMs are finished on every page they add areas to.

        This is wrong, not intended. I guess the is-first/is-last traits (from the spec) need to be introduced on the area tree to handle that properly.

        <snip/>

        Show
        Jeremias Maerki added a comment - (In reply to comment #3) > Further notes: > In the related code, there's some preparations for resolving the > page-number-citation-last correctly, but the problem is that this depends on > calls to notifyEndOfLayout(). > > Now, checking for blocks, that call gets made with every addAreas() call. Still > have to figure out whether this was intended or necessary for some reason, but > currently this means that the IDTracker mistakenly gets the impression that the > LMs are finished on every page they add areas to. This is wrong, not intended. I guess the is-first/is-last traits (from the spec) need to be introduced on the area tree to handle that properly. <snip/>
        Hide
        Andreas L. Delmelle added a comment -

        Thanks for the confirmation.

        In the meantime, I succeeded in getting the resolution for blocks correct, by modifying BlockLM.addAreas().

        Instead of simply 'notifyEndOfLayout()', I added:

        if (this.isLast(lastPos))

        { notifyEndOfLayout(); }

        'lastPos' is set a bit higher up to the last position returned by the position-iterator.

        This seems to do the trick.

        Now, I'm a bit wary of adding the same check to /all/ related LMs (code-duplication), so will try to implement it in a way that this can be handled by the superclass BlockStackingLM. That way, the problem can be solved for all the related LMs in one go.
        Implementing it in AbstractLM could, at the same time, solve the problem for inlines.

        Something like:

        AbstractLayoutManager.checkEndOfLayout(Position pos) {
        if (this.isLast(lastPos)

        { notifyEndOfLayout(); }

        }

        which can then be called by the subclasses, instead of directly calling notifyEndOfLayout().

        Show
        Andreas L. Delmelle added a comment - Thanks for the confirmation. In the meantime, I succeeded in getting the resolution for blocks correct, by modifying BlockLM.addAreas(). Instead of simply 'notifyEndOfLayout()', I added: if (this.isLast(lastPos)) { notifyEndOfLayout(); } 'lastPos' is set a bit higher up to the last position returned by the position-iterator. This seems to do the trick. Now, I'm a bit wary of adding the same check to /all/ related LMs (code-duplication), so will try to implement it in a way that this can be handled by the superclass BlockStackingLM. That way, the problem can be solved for all the related LMs in one go. Implementing it in AbstractLM could, at the same time, solve the problem for inlines. Something like: AbstractLayoutManager.checkEndOfLayout(Position pos) { if (this.isLast(lastPos) { notifyEndOfLayout(); } } which can then be called by the subclasses, instead of directly calling notifyEndOfLayout().
        Hide
        Andreas L. Delmelle added a comment -

        More notes:

        • page-number-citation seems to suffer the same limitations for block-container as for inline. If the citation ends up on the same page, the id resolution for the first page somehow doesn't get triggered. On subsequent pages spanned by the block-container, everything works fine.
        • block-container has the added difficulty of having to deal with border- and spacing-elements coming from the childLMs. BlockContainerLM no longer gets to see those positions when its addAreas() is called the last time. If it generates a list of 39 positions, the last two will be border and spacing for the last childLM, but those will not be included in the position iterator that is passed to addAreas(). As a result, the isLast() test will always return false in this case.

        Misleading javadoc for isLast(), I think:

        /**

        • Indicates whether the given Position is the last area-generating Position of this LM.

        But a border- or spacing-position does not really generate any areas. Rather, they belong to an area.

        Will need to check that as well, since the isLast() condition is also used to determine whether the markers associated with an area qualify for the retrieve-position of retrieve-marker.

        Show
        Andreas L. Delmelle added a comment - More notes: page-number-citation seems to suffer the same limitations for block-container as for inline. If the citation ends up on the same page, the id resolution for the first page somehow doesn't get triggered. On subsequent pages spanned by the block-container, everything works fine. block-container has the added difficulty of having to deal with border- and spacing-elements coming from the childLMs. BlockContainerLM no longer gets to see those positions when its addAreas() is called the last time. If it generates a list of 39 positions, the last two will be border and spacing for the last childLM, but those will not be included in the position iterator that is passed to addAreas(). As a result, the isLast() test will always return false in this case. Misleading javadoc for isLast(), I think: /** Indicates whether the given Position is the last area-generating Position of this LM. But a border- or spacing-position does not really generate any areas. Rather, they belong to an area. Will need to check that as well, since the isLast() condition is also used to determine whether the markers associated with an area qualify for the retrieve-position of retrieve-marker.
        Hide
        Andreas L. Delmelle added a comment -

        (In reply to comment #6)
        > More notes:
        > * page-number-citation seems to suffer the same limitations for block-container
        > as for inline. If the citation ends up on the same page, the id resolution for
        > the first page somehow doesn't get triggered. On subsequent pages spanned by
        > the block-container, everything works fine.

        Strike that. This was caused by the changes I had already done locally. The reason for this is precisely that the isLast() condition fails, and so notifyEndOfLayout() was not called anymore...

        It seems we could do with a distinction between lastGeneratedPosition and lastGeneratedBorderPaddingSpacePosition... well, something like that ;-P

        (TBC)

        Show
        Andreas L. Delmelle added a comment - (In reply to comment #6) > More notes: > * page-number-citation seems to suffer the same limitations for block-container > as for inline. If the citation ends up on the same page, the id resolution for > the first page somehow doesn't get triggered. On subsequent pages spanned by > the block-container, everything works fine. Strike that. This was caused by the changes I had already done locally. The reason for this is precisely that the isLast() condition fails, and so notifyEndOfLayout() was not called anymore... It seems we could do with a distinction between lastGeneratedPosition and lastGeneratedBorderPaddingSpacePosition... well, something like that ;-P (TBC)
        Hide
        Andreas L. Delmelle added a comment -

        Fixed in FOP Trunk

        see: http://svn.apache.org/viewvc?rev=654946&view=rev

        A surprisingly simple fix in the end... I'll see if this can be included in 0.95 final too, if nobody minds.

        Support for page-number-citations is far from complete, but I'm closing this issue since it was restricted to block and inline, which now work OK.

        Thanks for reporting!

        Show
        Andreas L. Delmelle added a comment - Fixed in FOP Trunk see: http://svn.apache.org/viewvc?rev=654946&view=rev A surprisingly simple fix in the end... I'll see if this can be included in 0.95 final too, if nobody minds. Support for page-number-citations is far from complete, but I'm closing this issue since it was restricted to block and inline, which now work OK. Thanks for reporting!
        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:
            Guillaume Levrero
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development