Bug 44794 - page-number-citation shows no value or incorrect value.
Summary: page-number-citation shows no value or incorrect value.
Status: CLOSED FIXED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: pdf (show other bugs)
Version: trunk
Hardware: PC Windows 2000
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-10 01:20 UTC by Guillaume Levrero
Modified: 2012-04-01 07:00 UTC (History)
0 users



Attachments
FO to reproduce bug. (2.26 KB, text/plain)
2008-04-10 01:20 UTC, Guillaume Levrero
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Guillaume Levrero 2008-04-10 01:20:32 UTC
Created attachment 21804 [details]
FO to reproduce bug.

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)
Comment 1 Andreas L. Delmelle 2008-05-08 15:29:04 UTC
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.
Comment 2 Andreas L. Delmelle 2008-05-08 16:49:05 UTC
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.
Comment 3 Andreas L. Delmelle 2008-05-09 02:40:02 UTC
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'.
Comment 4 Jeremias Maerki 2008-05-09 02:47:08 UTC
(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/>
Comment 5 Andreas L. Delmelle 2008-05-09 04:25:53 UTC
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().
Comment 6 Andreas L. Delmelle 2008-05-09 06:14:19 UTC
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.
Comment 7 Andreas L. Delmelle 2008-05-09 06:50:47 UTC
(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)
Comment 8 Andreas L. Delmelle 2008-05-09 14:45:26 UTC
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!
Comment 9 Glenn Adams 2012-04-01 07:00:26 UTC
batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed