Two test cases added to our test suite:
http://svn.apache.org/viewcvs?rev=348138&view=rev
The same problem exists with lists. The problem is the following:
In table and list layout Knuth element lists are combined to a single element
list. The footnotes contained in KnuthBlockBoxes in the original element lists
currently don't get propagated to the combined element lists and that's why the
footnotes get ignored. It shouldn't be too hard to fix.

The comitted patch causes footnotes in lists and tables to survive
(longer?) than without.
It is not finished, but I am at the limit of my knowledge
and tomorrow my usual business starts, so I want to
contribute what I have now - hoping it is a bit usefull
for others who continue or better rewrite.
Description:
A) Propagate footnotes in ListItemLayoutManger when
combined knuth elements are created.
B) Propagate footnotes in TableStepper when
combined knuth elements are created.
C) Propagate footnotes in TableContentLayoutManger when
boxes for table-header and table-footer are created.
The shortcomings:
1) Footnotes in list-item-label produce a
"Cannot find LM to handle given FO for LengthBase."
AFAICS in the getBaseLength method of AbstractBaseLayoutManger.
2) Footnotes from list-item-body starts at the same position
(from the starting edge) than the list-item-body itself and
not at the starting edge of the region-body.
3) Footnotes on table-header and table-footer are not printed
on every page where the content of the table-header and
table-footer is printed but only on that page that holds
the end of the table.
4) Footnotes on table-header and table-footer are ordered after
the footnotes of table-body.
5) There is one situation with my example fo-file for tables that
produce an endless loop (see comments there).

I don't think there's any work-around. It's really a non-trivial problem with
our layout approach. As far as I know, nobody's working on this at the moment. I
you're fearless, you can try to do it yourself based on the work Gerhard Oettl
started.

Wouldn't be better to include at least Gerhard's patch into the code? Correct, me if I'm wrong, but it's
better to have limited functionality of list & tables footnotes than none.
Having footnotes in lists is simply necessary thing when writing bigger projects documentation.
Cheers.

Workaround as offered by Ron Van den Branden on fop-users@ :
A way in which the problem can be avoided, is by generating fo:footnote areas for those footnotes *outside* the areas of their
containing lists and tables. If those fo:footnote areas don't contain any text in their fo:inline footnote markers, the latter don't
show up in the inline text, while the fo:footnote-body does end up in the footnote region at the bottom of the page. [Note: this
use of footnotes is inspired by solutions to other vertical alignment issues like
<http://www.dpawson.co.uk/xsl/sect3/fofixedposn.html#d12878e43>]
Stylesheet-wise this involves a two-way treatment of footnotes inside lists and tables: 1) generate the footnote markers inline
(just a fo:inline containing the footnote marker suffices), 2) generate the fo:footnote areas for each of those footnotes out-of-
line, inside a fo:block after the affected fo:list-block / fo:table.
I'll illustrate with following code, in which the first footnote doesn't get rendered, while the second one does:
<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
<fo:layout-master-set>
<fo:simple-page-master master-name="simple" page-height="5in" page-width="5in">
<fo:region-body/>
</fo:simple-page-master>
</fo:layout-master-set>
<fo:page-sequence master-reference="simple">
<fo:flow flow-name="xsl-region-body">
<fo:list-block provisional-distance-between-starts="50pt" provisional-label-separation="10pt">
<fo:list-item>
<fo:list-item-label end-indent="label-end()">
<fo:block>label</fo:block>
</fo:list-item-label>
<fo:list-item-body start-indent="body-start()">
<fo:block>
<!-- This fo:block contains a 'regular' fo:footnote inside a fo:list-block. Note that the fo:footnote-body
doesn't get rendered in the output, due to bug 37579
(http://issues.apache.org/bugzilla/show_bug.cgi?id=37579) -->
List item with a footnote<fo:footnote>
<fo:inline font-size="60%" baseline-shift="super">1)</fo:inline>
<fo:footnote-body>
<fo:block start-indent="0.5cm" text-indent="-0.5cm">
<fo:inline font-size="60%" baseline-shift="super">1)</fo:inline> This footnote doesn't get rendered.</fo:block>
</fo:footnote-body>
</fo:footnote>.
</fo:block>
</fo:list-item-body>
</fo:list-item>
<fo:list-item>
<fo:list-item-label end-indent="label-end()">
<fo:block>label</fo:block>
</fo:list-item-label>
<fo:list-item-body start-indent="body-start()">
<fo:block>
<!-- this footnote is only marked inline by a fo:inline marker -->
List item with a footnote<fo:inline font-size="60%" baseline-shift="super">2)</fo:inline>.
</fo:block>
</fo:list-item-body>
</fo:list-item> </fo:list-block>
<!-- this block contains the fo:footnote areas for all separate footnotes in the previous fo:list-block -->
<fo:block>
<fo:footnote>
<!-- this fo:inline footnote marker is empty to avoid it getting output after the fo:list-block -->
<fo:inline/>
<fo:footnote-body>
<fo:block start-indent="0.5cm" text-indent="-0.5cm">
<fo:inline font-size="60%" baseline-shift="super">2)</fo:inline> This footnote does get rendered.</fo:block>
</fo:footnote-body>
</fo:footnote>
</fo:block>
</fo:flow>
</fo:page-sequence>
</fo:root>
Note that this approach will require some refinements. For example, for lists / tables that span multiple pages, the footnotes
will all end up before / after the affected list / table (depending on the placement of their containing block). In order to avoid
this, the fo:list-block / fo:table areas could be generated at the lower level of the list items, i.e. the input list / table will not
generate a fo:list-block / fo:table, but each list item / table row will. Each of those fo:list-block / fo:table areas can then be
followed by a fo:block containing the relevant fo:footnote area. Of course this produces a lot of one-item lists / one-row tables,
but it also guarantees that footnotes will show at the right page when they occur in long lists / tables. I haven't tested it in my
production XSL-FO stylesheets, and of course treatment of nested lists would demand further consideration, but I think the
principle works.

In the mean time, I have further investigated the approach mentioned in the previous comment, and documented this research in a blog post at <http://rvdb.wordpress.com/2008/03/07/rendering-footnotes-in-tables-and-lists-with-fop/>.
It describes how far it got me in circumventing the bug and indicates where problems remain. It also provides a link to a downloadable zip
package containing sample source XML documents and an XSLT stylesheet
implementing the techniques discussed. I hope this can a) help others
looking for a solution to this problem and b) lead to a better solution.

I'm attaching an updated patch, that can be applied to the trunk as it is now.
It basically does the same thing as the old one, just in a slightly different way due to the new classes involved in the table management.
The 5 "shortcomings" listed by Gerhard Oettl in comment #4 are still present, although I'm not sure whether the second one (indentation of notes whose citation is in the list-item-body) is a bug or the right behaviour, because of indent inheritance ...
I've tried to track down the LengthBase error message, but I could not find where the problem really is: the strange thing is that it happens only for footnotes in the list-item-label, and not for those in the list-item-body ...
Footnotes on table headers / footers will probably need a special handling, as they behave quite differently from the "normal" ones, being repeated.

(In reply to comment #16)
> I'm attaching an updated patch, that can be applied to the trunk as it is now.
Thanks! Good to see you back in action ;-)
>
> It basically does the same thing as the old one, just in a slightly different
> way due to the new classes involved in the table management.
>
> The 5 "shortcomings" listed by Gerhard Oettl in comment #4 are still present,
> although I'm not sure whether the second one (indentation of notes whose
> citation is in the list-item-body) is a bug or the right behaviour, because of
> indent inheritance ...
It is correct behavior. No special rules are defined for property-inheritance on fo:footnotes, so default inheritance applies. Stylesheet authors should reset the indent to zero on the fo:footnote, if so desired.
>
> I've tried to track down the LengthBase error message, but I could not find
> where the problem really is: the strange thing is that it happens only for
> footnotes in the list-item-label, and not for those in the list-item-body ...
The reason seems to occur due to resolution of the label-end() function that is triggered from within FootnoteBodyLM.
Looking at org.apache.fop.fo.expr.LabelEndFunction, there is a component PercentLength being created. Resolution of this percentage during normal layout is done by looking up the associated ancestor LM, but since (if I interpret correctly) the FootnoteBodyLM is not a descendant of the original ListItemLayoutManager, this resolution fails...
Setting the fo:footnote's end-indent to "0pt" eliminates the dependency on label-end() in the footnote, and thus also the warnings.
>
> Footnotes on table headers / footers will probably need a special handling, as
> they behave quite differently from the "normal" ones, being repeated.
>
Right, my first instinct would be to include footnotes for the table-header only on the first page that is spanned by the table, and for the table-footer only on the last page.

(In reply to comment #18)
> Just ran the layoutengine test-suite after having applied the patch, and it
> seems this causes quite a few tests to break. One of the reasons is an
> IndexOutOfBoundsException at TableStepper line#237...
>
Found the cause of this: the error was that the additional code in TableStepper assumed the list of activeCells to be always equal to the number of columns. Changing the code-fragment to be based on activeCells.size() instead of columnCount did the trick.
Also, but that's maybe more of a personal preference, IMO the ElementListUtils.collectFootnodeBodyLMs() implementations should better be turned around for reasons of code-clarity.
Right now, the implementation dealing with a single List creates one-element arrays and delegates to the implementation for an array of List. Somehow, I like the opposite where a call to the latter implementation expands into multiple calls to the 'simple' implementation.

Further investigation into the ordering of footnotes for table-header and table-footer revealed that they are added to the first/last page in case we add to the table:
table-omit-header-at-break="true"
The reason, if I judge correctly, is that in this case, the header's element-list is added to the head of the combined element list. In the default case, with the header repeated at each break, that list will be the next-to-last element in the combined list. The footer will be the last, if present.
The infinite loop seems to point to a borderline-case where there is 'just (not) enough' room to fit both the table and all its footnotes on one page. Further investigation pending...
The problem with getLengthBase() seems to point to a difficulty with property-inheritance: strictly speaking, the footnote should inherit the computed value for start-indent(), but during property resolution, this value is not yet known if the specified value is "label-end()". Since LabelEndFunction makes use of a PercentLength that can only be evaluated during layout, the descendants of the list-item-label inherit the expression:
reference-area-width - (provisional-distance-between-starts + start-indent - provisional-label-separation)

(In reply to comment #21)
> The problem with getLengthBase() seems to point to a difficulty with
> property-inheritance: strictly speaking, the footnote should inherit the
> computed value for start-indent(),
Sorry, I obviously meant "end-indent".
That said, taking indent-inheritance into account, *not* resetting end-indent on an fo:footnote that is a descendant of an fo:list-item-label is very likely to count as 'bad practice'.

Thank you for all your comments and additions, Andreas!
It's good to be back after quite a long time (enough to forget the good old habit of using JUnit!)
(In reply to comment #22)
> (In reply to comment #21)
> > The problem with getLengthBase() seems to point to a difficulty with
> > property-inheritance: strictly speaking, the footnote should inherit the
> > computed value for start-indent(),
>
> Sorry, I obviously meant "end-indent".
What I still cannot understand is why there is no such problem with start-indent = "body-start", which is resolved ok ...
(In reply to comment #17)
> Right, my first instinct would be to include footnotes for the table-header
> only on the first page that is spanned by the table, and for the table-footer
> only on the last page.
It's an interesting idea, and probably the easiest to implement.
Personally, I would have placed them all in the first page spanned by the table, although this would be a bit more problematic in terms of relative order between the footnotes.
What do other think in this regard?
Anyway, I think this is a situation not perfectly covered by the specs, which forbid footnotes in static-contents but say nothing about footnotes in table headers / footers, which are not so different.
The condition defining where a block area returned by fo:footnote is permitted does not explicitly take into account the situation when a single fo:foonote generates several anchor areas in different pages, although the definition
"The term anchor-area is defined to mean the last area that is generated and returned by the fo:inline child of the fo:footnote."
could maybe be read as a justification for placing all the notes in the last page where the table appears ...

(In reply to comment #23)
> Thank you for all your comments and additions, Andreas!
> It's good to be back after quite a long time (enough to forget the good old
> habit of using JUnit!)
:-) No problem. That's why it's A Good Thing that there's more of us.
> > (In reply to comment #21)
> > > The problem with getLengthBase() seems to point to a difficulty with
> > > property-inheritance: strictly speaking, the footnote should inherit the
> > > computed value for start-indent(),
> >
> > Sorry, I obviously meant "end-indent".
>
> What I still cannot understand is why there is no such problem with
> start-indent = "body-start", which is resolved ok ...
They are implemented differently: see org.apache.fop.fo.expr.BodyStartFunction and .LabelEndFunction. label-end() makes explicit use of a percentage, body-start() does not...
It is only that percentage which causes the error. For 'normal' list-item-label descendants, the base-length is found by climbing up the LM tree until the corresponding LM is found. That's what happens in AbstractBaseLM.getBaseLength().
Come to think of it, maybe a way to avoid the warning would be to reimplement getBaseLength() in FootnoteBodyLM, to do something else than try out all ancestors... (long shot)
> > Right, my first instinct would be to include footnotes for the table-header
> > only on the first page that is spanned by the table, and for the table-footer
> > only on the last page.
>
> It's an interesting idea, and probably the easiest to implement.
> Personally, I would have placed them all in the first page spanned by the
> table, although this would be a bit more problematic in terms of relative order
> between the footnotes.
Yours is probably the better idea... Since the footer can appear on the first page, it would indeed be confusing to have its footnote appear maybe 10 pages after the citation first appears.

(In reply to comment #24)
> ...
> It is only that percentage which causes the error. For 'normal' list-item-label
> descendants, the base-length is found by climbing up the LM tree until the
> corresponding LM is found. That's what happens in
> AbstractBaseLM.getBaseLength().
> Come to think of it, maybe a way to avoid the warning would be to reimplement
> getBaseLength() in FootnoteBodyLM, to do something else than try out all
> ancestors... (long shot)
Did some further research, and the ancestor tree for both footnotes' LMs (label and body) looks like:
ListItemContentLayoutManager
-> BlockLayoutManager
-> LineLayoutManager
-> FootnoteLayoutManager
-> FootnoteBodyLayoutManager
The default getBaseLength() in percentage resolution relies on the LM's getFObj() method to find the LM corresponding to the FO on which the percentage is based (the one that is passed as an argument to the PercentLength constructor in LabelEndFunction).
For the ListItemContentLayoutManager, this method always returns the ListItemBody (never the ListItemLabel).
It would probably work as expected if we had separate ListItemLabelLM and ListItemBodyLM. Similar issues occur with the table-related objects BTW, where we also do not have a 1-1 correspondence between FO and LM...

(In reply to comment #26)
>
> Strike that... Stopped debugging too early. The label's LM does appear if I
> leave it running...
>
At the point where getBaseLength() fails, the ancestor tree looks like:
FlowLayoutManager
-> FootnoteBodyLM
So, the LM is reparented (in PageBreaker, line 166), and somewhere after that, we get a call to PercentLength.getValue(footnoteBodyLayoutManager)

Created attachment 21922[details]
Patch for FootnoteBodyLayoutManager
Naively tried adding an override for getParent() to FootnoteBodyLM that always returns the associated FootnoteLM, and then it works without errors, but as I expected, the footnote's end-indent is then equal to that of the label. May count as a fix, though...
I haven't checked yet if there's a situation where FootnoteBodyLM.getParent() is supposed to return the FlowLM.

In the meantime, I managed to track down the point of origin for the infinite loop. No solution yet, but it happens in PageBreakingAlgorithm.createFootnotePages(), when it is called for the second page (which I presume to be a footnote-only page).
The member 'insertedFootnotesLength' never changes, and it is less than the 'totalFootnotesLength', so the loop body is never exited.
Adding a third variable to check whether the first value has actually changed compared to the previous iteration, makes the loop finish, but apparently triggers creation of so many nodes, that no matter how much heap you give it, it still runs out of memory...

Good news: No idea why exactly, but I just tried again to reproduce the infinite loop with the latest trunk, and couldn't. Maybe has something to do with the changes Vincent has committed to the table-layout code earlier today (?)
The bad news is that the footnote in the table-footer is now skipped/swallowed...

Just had another look, and it seems to have been a fluke... :(
The infinite loop remains. Debugging it, I can almost 'see' what's going wrong, but any attempt to bypass it so far, have failed. If not for this case, then for the unit-tests for split footnotes.
The story goes:
-> after the page-breaking loop completes, the first page is finished, and its footnotes added as much as possible (4 footnotes in this case)
-> after that, we see there's still one footnote left which has its citation on that page (PBA.finish(): (node.totalFootnotes < PBA.totalFootnotesLength))
-> so createFootnotePages() is called, which starts at the last footnote that was added to the previous page; could be a split footnote
=> We hang indefinitely in:
...
while (insertedFootnotesLength < totalFootnotesLength) {
tmpLength = ((Integer)lengthList.get(footnoteListIndex)).intValue();
// try adding some more content
if ((tmpLength - insertedFootnotesLength) <= availableBPD) {
// add a whole footnote
availableBPD -= tmpLength - insertedFootnotesLength;
insertedFootnotesLength = tmpLength;
footnoteElementIndex
= ((LinkedList)footnotesList.get(footnoteListIndex)).size() - 1;
...
...
}
insertedFootnotesLength never changes, availableBPD always remains the same (tmpLength == insertedFootnotesLength), so the condition to break the while-loop is never reached.

In the meantime, I also compared the behavior of the table-testcase with our layout-test for footnote splits.
The one notable difference between the two cases: if a footnote appears in a one line block, and none of the lines of the footnote fit on the page, then the entire block is moved. If there is a keep-with-previous.within-page="always" on the block, then the preceding block is moved along. Only if at least one line of the footnote fits, a part of the block is also put on the first page.
Seems like something similar is happening with the footnote in the table-footer. The footnote does not fit, not even one line, but the table-footer cannot be moved (either completely or partially) to the next page either.

(In reply to comment #32)
> The footnote does not fit, not even one line, but the
> table-footer cannot be moved (either completely or partially) to the next page
> either.
Brilliant investigation, Andreas!
So there are two main differences between "normal" footnotes and those in table-header / footer (when table-omit-*-at-break = false), as the latters:
- need to be repeated in each page where their related table appears
- cannot be "delayed" to the next page as this would lead to a duplication, nor they can be moved together with the header
From the implementation point of view, this seems to suggest that, as the table-header (or footer) and its footnotes are actually indivisible (*), their overall length could be used to generate the table elements ... altough this would not be 100% accurate as the proper space resolution with the other surrounding footnotes would be lost.
Mmhh, I need to think about this some more time ...
(*) Well, there is at least a case when partially deferring table-footer footnotes would not be completely ugly: when the table content is finished, so that there would not be footnote repetition in the next page

Update:
Adrian contacted me off-list to see if we could at least partially apply the attached patches.
My proposal would be to integrate the changes for footnote support in lists and the table-body, but leave headers/footers out for the moment. This restriction should obviously be reflected in the documentation.
Unless any fop-devs object to this, I'll go ahead, create some additional checks in the existing jUnit tests, update the docs, and apply those changes in the weekend.

I think this patch should only be applied when it is ready, looks like there is still quite a bit of cleanup to do. Lets try and have a model that encapsulates a complete solution to the problem in all cases otherwise supporting this feature will be more difficult and confusing for users. I think ElementListUtils.collectFootnoteBodyLMs() could be revised somewhat.
(In reply to comment #34)
> Update:
> Adrian contacted me off-list to see if we could at least partially apply the
> attached patches.
> My proposal would be to integrate the changes for footnote support in lists and
> the table-body, but leave headers/footers out for the moment. This restriction
> should obviously be reflected in the documentation.
>
> Unless any fop-devs object to this, I'll go ahead, create some additional
> checks in the existing jUnit tests, update the docs, and apply those changes in
> the weekend.
>

(In reply to comment #35)
> I think this patch should only be applied when it is ready, looks like there is
> still quite a bit of cleanup to do. Lets try and have a model that
> encapsulates a complete solution to the problem in all cases otherwise
> supporting this feature will be more difficult and confusing for users. I
> think ElementListUtils.collectFootnoteBodyLMs() could be revised somewhat.
I'd be happy to look into that. Can you be more specific? The method in question is a general-purpose utility method that can potentially be used by all related LayoutManagers to extract the list of footnotes from an element-list generated by one of their descendants. I don't really see what could or should be revised, so if you have any suggestions...
It works like a charm anyway, apart from the mentioned anomaly with table-footers.
Of course, we could also leave it another two years... ;-P

(In reply to comment #36)
> (In reply to comment #35)
> > I think this patch should only be applied when it is ready, looks like there is
> > still quite a bit of cleanup to do. Lets try and have a model that
> > encapsulates a complete solution to the problem in all cases otherwise
> > supporting this feature will be more difficult and confusing for users. I
> > think ElementListUtils.collectFootnoteBodyLMs() could be revised somewhat.
>
> I'd be happy to look into that. Can you be more specific? The method in
> question is a general-purpose utility method that can potentially be used by
> all related LayoutManagers to extract the list of footnotes from an
> element-list generated by one of their descendants. I don't really see what
> could or should be revised, so if you have any suggestions...
>
> It works like a charm anyway, apart from the mentioned anomaly with
> table-footers.
I'm sorry to chime in so late, but so far I haven't had the time and energy to approach this topic.
Anyway, I've just had a look at the patch and I'm afraid I don't think it's ready to be committed.
I see mainly the following problems:
- from a high-level point of view first: list- and table-related code should remain totally footnote-agnostic. The footnote-handling code should remain in a single class and not be spread over the codebase, which would make it error-prone and difficult to maintain and understand.
- not sure the collectFootnoteBodyLMs taking array parameters is any useful. In the calling code a new array is created and populated with the element lists to parse, and in the method this array is iterated over to get back to the single element lists...
Below I will only speak for the table code, because it's the only one in the code impacted by the patch that I know well, but I think the changes don't fit well in it:
- in TableStepper, ActiveCells aren't ordered by their column indices! Instead they are appended to the list once they start contributing content for the merging algorithm. This means that new cells will be found /after/ cells starting on earlier rows and spanning over the current one, even if they are in earlier columns. So basically the code added in TableStepper doesn't work...
- in CellPart, there's no reason why the getStartIndex and getEndIndex methods should be made public, package-private would be enough. But in the first place footnotes shouldn't really be handled that way. There is a negative impact on performance since at every iteration, the cells' (linked) lists of elements will be re-skimmed through from the beginning up to the start index.
An intermediate solution could probably be implemented the following way:
- in ActiveCell.Step, add a List field that would contain the FootnoteBodyLMs encountered during the last call to gotoNextLegalBreak;
- in TableStepper.getCombinedKnuthElements, when iterating over the active cells to create the CellPart instances, get those FootnoteBodyLMs in the same time. A small detail to pay attention to is to not re-get them if the active cell doesn't contribute new content to the step. If there are some footnotes, create a KnuthBlockBox, otherwise create a normal Box.
And that should be it basically...
I'll see if I can submit a patch to illustrate my ideas in the next days.
Vincent

(In reply to comment #37)
>
> I'm sorry to chime in so late, but so far I haven't had the time and energy to
> approach this topic.
No problem.
>
> Anyway, I've just had a look at the patch and I'm afraid I don't think it's
> ready to be committed.
> I see mainly the following problems:
> - from a high-level point of view first: list- and table-related code should
> remain totally footnote-agnostic. The footnote-handling code should remain in a
> single class and not be spread over the codebase, which would make it
> error-prone and difficult to maintain and understand.
Now that you mention it...
The current implementation has the related code (footnote gathering) in PageBreaker.getNextKnuthElements(). The only reason so far that it didn't work for lists and tables is that they did not yet propagate the footnotes for their descendants upwards.
I'm wondering how can this be done without making either of them footnote-aware... They are agnostic, that is precisely why it does not work now. Both generate combined element-lists for their parts, but the parts' footnotes are not picked up.
> Below I will only speak for the table code, because it's the only one in the
> code impacted by the patch that I know well, but I think the changes don't fit
> well in it:
Good! That's why I was hoping you'd chime in.
> - in TableStepper, ActiveCells aren't ordered by their column indices! Instead
> they are appended to the list once they start contributing content for the
> merging algorithm. This means that new cells will be found /after/ cells
> starting on earlier rows and spanning over the current one, even if they are in
> earlier columns. So basically the code added in TableStepper doesn't work...
OK, I see where this becomes a problem. Had not yet tested such cases, only simple grids.
> An intermediate solution could probably be implemented the following way:
> - in ActiveCell.Step, add a List field that would contain the FootnoteBodyLMs
> encountered during the last call to gotoNextLegalBreak;
> - in TableStepper.getCombinedKnuthElements, when iterating over the active
> cells to create the CellPart instances, get those FootnoteBodyLMs in the same
> time. A small detail to pay attention to is to not re-get them if the active
> cell doesn't contribute new content to the step. If there are some footnotes,
> create a KnuthBlockBox, otherwise create a normal Box.
> And that should be it basically...
> I'll see if I can submit a patch to illustrate my ideas in the next days.
Cool, we await your input.

(In reply to comment #38)
<snip />
> Now that you mention it...
> The current implementation has the related code (footnote gathering) in
> PageBreaker.getNextKnuthElements().
To be complete: the first steps are taken in LineLayoutManager.postProcessLineBreaks().

(In reply to comment #38)
>
> > An intermediate solution could probably be implemented the following way:
> > - in ActiveCell.Step, add a List field that would contain the FootnoteBodyLMs
> > encountered during the last call to gotoNextLegalBreak;
> > - in TableStepper.getCombinedKnuthElements, when iterating over the active
> > cells to create the CellPart instances, get those FootnoteBodyLMs in the same
> > time. A small detail to pay attention to is to not re-get them if the active
> > cell doesn't contribute new content to the step. If there are some footnotes,
> > create a KnuthBlockBox, otherwise create a normal Box.
> > And that should be it basically...
> > I'll see if I can submit a patch to illustrate my ideas in the next days.
>
> Cool, we await your input.
Well, I did not really wait... ;-)
I've already tried this approach, and I got this working, apart from 'not re-get them if they do not contribute content'. If you could tell me what condition I need to check for, that would save me the time to go looking.
Right now, I have:
-> added a footnoteList member to ActiveCell, with accompanying accessor
-> modified gotoNextLegalBreak(): if the element is a KnuthBlockBox and has anchors, then add the footnotes to the added member-list
-> modified TableStepper (loop +/- line 207) to use the accessor; if the ActiveCell has footnotes, a KnuthBlockBox is generated further on, else a normal Box.
Thanks for the helpful feedback so far! It's giving me better insight into table-layout as well, which could come in handy at some point. ;-)

(In reply to comment #41)
> Created an attachment (id=21976) [details]
> alternative patch for footnotes in table-cells
>
>
> If I'm correct, this should be roughly what Vincent proposed (?)
>
Just noticed: after this patch, there does not seem to be a good reason anymore to have the ElementListUtils.collectFootnoteBodyLMs() methods. The only class accessing it, is ListItemLayoutManager, so we may as well put it there in a private method, or inline... Updated patch to follow.

Created attachment 21977[details]
updated patch against FOP trunk
Updated patch, without support for footnotes in table-header or -footer, which removes ElementListUtils from the picture.
For list-items, I also took the liberty of making one more modification. Not sure if this agrees with everyone, but if the idea is to improve element access performance in getNextStep(), and we're not modifying the lists anyway, we might as well switch to plain arrays instead of copying the LinkedLists to ArrayLists.

(In reply to comment #38)
> (In reply to comment #37)> > - from a high-level point of view first: list- and table-related code should
> > remain totally footnote-agnostic. The footnote-handling code should remain in a
> > single class and not be spread over the codebase, which would make it
> > error-prone and difficult to maintain and understand.
>
<snip />
> I'm wondering how can this be done without making either of them
> footnote-aware...
Been doing some more thinking, and what if we were to introduce something like a 'FootnoteCollector'? I think something like this would also address Adrian's concern about a complete solution...
Right now, the LineLayoutManager separates the footnotes from their citations, and attaches them as a member-list to the KnuthBlockBoxes. The same approach is now copied to list- and table-layout: extraction of the footnotes from the boxes, and copying them to higher-level block-boxes.
What if we pass a FootnoteCollector down from the PageBreaker, which contains a Map<KnuthBox, List<FootnoteBodyLM>>, or maybe simply Map<KnuthBox, FootnoteBodyLM[]>.
The LineLayoutManager would do something like:
footnoteCollector.collect( KnuthBox, List<KnuthBox> );
which would use the box as a key, and put the resulting list as a value in the map.
Something similar would be done by the list- and table-related LMs.
The iterations that are now spread over LineLM, PageBreaker, ActiveCell, TableStepper and ListItemLM can then be confined to one single class.
PageBreaker could do something like:
footnoteCollector.getFootnotesFor( List<KnuthBox> )
to obtain a combined list of footnotes for all the boxes in the list.
WDYT?

(In reply to comment #43)
> Created an attachment (id=21977) [details]
> updated patch against FOP trunk
>
>
> Updated patch, without support for footnotes in table-header or -footer, which
> removes ElementListUtils from the picture.
>
> For list-items, I also took the liberty of making one more modification. Not
> sure if this agrees with everyone, but if the idea is to improve element access
> performance in getNextStep(), and we're not modifying the lists anyway, we
> might as well switch to plain arrays instead of copying the LinkedLists to
> ArrayLists.
>
(In reply to comment #40)
> (In reply to comment #38)
> >
> > > An intermediate solution could probably be implemented the following way:
> > > - in ActiveCell.Step, add a List field that would contain the FootnoteBodyLMs
> > > encountered during the last call to gotoNextLegalBreak;
> > > - in TableStepper.getCombinedKnuthElements, when iterating over the active
> > > cells to create the CellPart instances, get those FootnoteBodyLMs in the same
> > > time. A small detail to pay attention to is to not re-get them if the active
> > > cell doesn't contribute new content to the step. If there are some footnotes,
> > > create a KnuthBlockBox, otherwise create a normal Box.
> > > And that should be it basically...
> > > I'll see if I can submit a patch to illustrate my ideas in the next days.
> >
> > Cool, we await your input.
>
> Well, I did not really wait... ;-)
> I've already tried this approach, and I got this working, apart from 'not
> re-get them if they do not contribute content'. If you could tell me what
> condition I need to check for, that would save me the time to go looking.
>
> Right now, I have:
> -> added a footnoteList member to ActiveCell, with accompanying accessor
> -> modified gotoNextLegalBreak(): if the element is a KnuthBlockBox and has
> anchors, then add the footnotes to the added member-list
> -> modified TableStepper (loop +/- line 207) to use the accessor; if the
> ActiveCell has footnotes, a KnuthBlockBox is generated further on, else a
> normal Box.
This is already much better, but this can still be improved IMO. TableStepper is still taking too much responsibility: instead of asking ActiveCells if they have any footnotes, and then asking them for their footnotes, it can just provide them with a list to which they can add their own footnotes, if they have any, and if they contribute content to the next step. Kind of inversion of control principle.
> Thanks for the helpful feedback so far! It's giving me better insight into
> table-layout as well, which could come in handy at some point. ;-)
>
Vincent

Created attachment 21979[details]
Updated patch, TableStepper delegating more to ActiveCell
Patch showing how TableStepper can delegate more responsibility to ActiveCell. I didn't touch the modification to list-related class. However, I reverted the modifications to FootnoteBodyLM since they were breaking the testsuite.

(In reply to comment #43)
> Created an attachment (id=21977) [details]
> updated patch against FOP trunk
>
>
> Updated patch, without support for footnotes in table-header or -footer, which
> removes ElementListUtils from the picture.
>
> For list-items, I also took the liberty of making one more modification. Not
> sure if this agrees with everyone, but if the idea is to improve element access
> performance in getNextStep(), and we're not modifying the lists anyway, we
> might as well switch to plain arrays instead of copying the LinkedLists to
> ArrayLists.
I don't really mind, but this should be done separately from the implementation of footnotes (atomicity of commits).
Vincent

(In reply to comment #27)
> (In reply to comment #26)
> >
> > Strike that... Stopped debugging too early. The label's LM does appear if I
> > leave it running...
> >
>
> At the point where getBaseLength() fails, the ancestor tree looks like:
>
> FlowLayoutManager
> -> FootnoteBodyLM
>
> So, the LM is reparented (in PageBreaker, line 166), and somewhere after that,
> we get a call to PercentLength.getValue(footnoteBodyLayoutManager)
AFAIU the reason why FootnoteBodyLM is re-parented is that it put its areas at the right place (as children of the footnote-reference-area, instead of the block containing the footnote). Simply moving the setParent call after the call to getNextKnuthElements makes the warnings disappear, and doesn't break any test.
Confirmation from specialists of this part of the code would be appreciated ;-)
Vincent

(In reply to comment #48)
>
> AFAIU the reason why FootnoteBodyLM is re-parented is that it put its areas at
> the right place (as children of the footnote-reference-area, instead of the
> block containing the footnote). Simply moving the setParent call after the call
> to getNextKnuthElements makes the warnings disappear, and doesn't break any
> test.
> Confirmation from specialists of this part of the code would be appreciated ;-)
Seems reasonable to me. Cleaner than overriding getParent() anyway.
In the meantime, I've also been playing with adding an interface FootnoteCitationHolder. Such an interface could then be implemented by KnuthBlockBox and ActiveCell. The interface methods can be used by LineLayoutManager, PageBreaker, ListItemLayoutManager, TableStepper...
The methods would roughly be:
hasAnchors()
getFootnoteBodyLMs()
addFootnotes(List<KnuthElement>)
addFootnotes(FootnoteCitationHolder)
addFootnote(FootnoteCitationHolder.Citation)
expandFootnotes(LayoutManager, LayoutContext, int)
While this would still leave the related portions of code distributed over those classes, the interface makes it a bit easier to locate them in an IDE, and makes those pieces of code a bit more uniform.
Most of the loops we see now, would move to KnuthBlockBox, as the only complete implementation. ActiveCell would only implement what is needed to make the citations accessible to the box created by TableStepper. Slight compromise in comparison to the last patch is that, in the iteration over the active cells, we would only create a temporary list with those having citations. If the list is empty, we create a regular box. If not, then we iterate over that temporary list of cells, and instruct the created KnuthBlockBox to add the citations from those cells. The same pattern can be used by ListItemLayoutManager:
- create a temporary list of FootnoteCitationHolders for which hasAnchors() returns true
- if non-empty, iterate over that temporary list
- for each element, ask the higher level FootnoteCitationHolder (KnuthBlockBox) to extract the citations, and add them to its own list.
I'll see if I can attach a patch to demonstrate one of these days.

(In reply to comment #49)
> (In reply to comment #48)
> >
> > AFAIU the reason why FootnoteBodyLM is re-parented is that it put its areas at
> > the right place (as children of the footnote-reference-area, instead of the
> > block containing the footnote). Simply moving the setParent call after the call
> > to getNextKnuthElements makes the warnings disappear, and doesn't break any
> > test.
> > Confirmation from specialists of this part of the code would be appreciated ;-)
>
> Seems reasonable to me. Cleaner than overriding getParent() anyway.
>
> In the meantime, I've also been playing with adding an interface
> FootnoteCitationHolder. Such an interface could then be implemented by
> KnuthBlockBox and ActiveCell. The interface methods can be used by
> LineLayoutManager, PageBreaker, ListItemLayoutManager, TableStepper...
>
> The methods would roughly be:
> hasAnchors()
> getFootnoteBodyLMs()
> addFootnotes(List<KnuthElement>)
> addFootnotes(FootnoteCitationHolder)
> addFootnote(FootnoteCitationHolder.Citation)
> expandFootnotes(LayoutManager, LayoutContext, int)
>
> While this would still leave the related portions of code distributed over
> those classes, the interface makes it a bit easier to locate them in an IDE,
> and makes those pieces of code a bit more uniform.
>
> Most of the loops we see now, would move to KnuthBlockBox, as the only complete
> implementation. ActiveCell would only implement what is needed to make the
> citations accessible to the box created by TableStepper. Slight compromise in
> comparison to the last patch is that, in the iteration over the active cells,
> we would only create a temporary list with those having citations. If the list
> is empty, we create a regular box. If not, then we iterate over that temporary
> list of cells, and instruct the created KnuthBlockBox to add the citations from
> those cells. The same pattern can be used by ListItemLayoutManager:
>
> - create a temporary list of FootnoteCitationHolders for which hasAnchors()
> returns true
> - if non-empty, iterate over that temporary list
> - for each element, ask the higher level FootnoteCitationHolder (KnuthBlockBox)
> to extract the citations, and add them to its own list.
>
> I'll see if I can attach a patch to demonstrate one of these days.
I've been asked to look into this issue, so I committed a partial and temporary fix based on the latest patch:
http://svn.apache.org/viewvc?view=rev&revision=660979
Footnotes in table headers and footers are not handled yet, and anyway I think it's best to wait for clarification from xsl-editors before implementing anything (which gives us a couple of months ;-) ).
That doesn't prevent you from exploring your ideas above, though. I await your patch.
Vincent

(In reply to comment #48)
> (In reply to comment #27)
> > (In reply to comment #26)
> > At the point where getBaseLength() fails, the ancestor tree looks like:
> >
> > FlowLayoutManager
> > -> FootnoteBodyLM
> >
> > So, the LM is reparented (in PageBreaker, line 166), and somewhere after that,
> > we get a call to PercentLength.getValue(footnoteBodyLayoutManager)
>
> AFAIU the reason why FootnoteBodyLM is re-parented is that it put its areas at
> the right place (as children of the footnote-reference-area, instead of the
> block containing the footnote). Simply moving the setParent call after the call
> to getNextKnuthElements makes the warnings disappear, and doesn't break any
> test.
This is more complicated than that. For most of the properties that can take a percentage value, the percentage refers to the containing area's (or nearest ancestor reference area's) ipd or bpd. The only notable exception is font-size, which refers to the parent element.
So if we take end-indent, for instance, if it's not specified inside footnote-body, then it takes the /computed/ value of the parent element (e.g., the block containing the footnote). /But/ if it has a specified percentage value, then the percentage shall be resolved against the footnote-reference-area. In many cases this will lead to the same result, but not when footnotes are declared inside lists, tables, block-containers, or if the region-body has several columns, etc.
This issue is not related to lists and tables only, but is more general. If PageBreaker is left as is, then percentages specified inside footnotes should resolve correctly, but not inherited values. If we move the setParent call like explained above, then this is the other way around...

(In reply to comment #50)
>
> I've been asked to look into this issue, so I committed a partial and temporary
> fix based on the latest patch:
> http://svn.apache.org/viewvc?view=rev&revision=660979
> Footnotes in table headers and footers are not handled yet, and anyway I think
> it's best to wait for clarification from xsl-editors before implementing
> anything (which gives us a couple of months ;-) ).
First, many thanks to all for your great efforts.
As I've seen, fop-0.95beta has been released by 2008-03-25 so these temporally fixes (2008-05-28) are still not present in it.
Is there any preliminar release date for beta2 or something like that?. A couple of months?. Should I use trunk?
Sorry for the noise.

(In reply to comment #52)
>
> As I've seen, fop-0.95beta has been released by 2008-03-25 so these temporally
> fixes (2008-05-28) are still not present in it.
Correct.
>
> Is there any preliminar release date for beta2 or something like that?. A
> couple of months?. Should I use trunk?
If you really need this feature, then you should indeed use the trunk. 0.95final is very close, but the changes have not applied to the release branch.

(In reply to comment #54)
> I believe this bug is now fixed. Isn't it ?
No. Footnotes have not been implemented yet in table headers and footnotes, as we are waiting for clarification from the W3C whether they should appear only once or every time the header/footer is repeated. Plus there currently are issues with percentages and inherited values inside footnotes (see comment #51 above).
Vincent

(In reply to comment #50)
> (In reply to comment #49)
> > (In reply to comment #48)
> > >
> > > AFAIU the reason why FootnoteBodyLM is re-parented is that it put its areas at
> > > the right place (as children of the footnote-reference-area, instead of the
> > > block containing the footnote). Simply moving the setParent call after the call
> > > to getNextKnuthElements makes the warnings disappear, and doesn't break any
> > > test.
> > > Confirmation from specialists of this part of the code would be appreciated ;-)
> >
> > Seems reasonable to me. Cleaner than overriding getParent() anyway.
> >
> > In the meantime, I've also been playing with adding an interface
> > FootnoteCitationHolder. Such an interface could then be implemented by
> > KnuthBlockBox and ActiveCell. The interface methods can be used by
> > LineLayoutManager, PageBreaker, ListItemLayoutManager, TableStepper...
> >
> > The methods would roughly be:
> > hasAnchors()
> > getFootnoteBodyLMs()
> > addFootnotes(List<KnuthElement>)
> > addFootnotes(FootnoteCitationHolder)
> > addFootnote(FootnoteCitationHolder.Citation)
> > expandFootnotes(LayoutManager, LayoutContext, int)
> >
> > While this would still leave the related portions of code distributed over
> > those classes, the interface makes it a bit easier to locate them in an IDE,
> > and makes those pieces of code a bit more uniform.
> >
> > Most of the loops we see now, would move to KnuthBlockBox, as the only complete
> > implementation. ActiveCell would only implement what is needed to make the
> > citations accessible to the box created by TableStepper. Slight compromise in
> > comparison to the last patch is that, in the iteration over the active cells,
> > we would only create a temporary list with those having citations. If the list
> > is empty, we create a regular box. If not, then we iterate over that temporary
> > list of cells, and instruct the created KnuthBlockBox to add the citations from
> > those cells. The same pattern can be used by ListItemLayoutManager:
> >
> > - create a temporary list of FootnoteCitationHolders for which hasAnchors()
> > returns true
> > - if non-empty, iterate over that temporary list
> > - for each element, ask the higher level FootnoteCitationHolder (KnuthBlockBox)
> > to extract the citations, and add them to its own list.
> >
> > I'll see if I can attach a patch to demonstrate one of these days.
>
> I've been asked to look into this issue, so I committed a partial and temporary
> fix based on the latest patch:
> http://svn.apache.org/viewvc?view=rev&revision=660979
> Footnotes in table headers and footers are not handled yet, and anyway I think
> it's best to wait for clarification from xsl-editors before implementing
> anything (which gives us a couple of months ;-) ).
>
> That doesn't prevent you from exploring your ideas above, though. I await your
> patch.
>
> Vincent
I try to use the trunk 660979 and find a case, when a footnote defined in table-body disappears. An example of this is attached. I look for solution since some days, but don’t get much with my very modest knowledge of fop. Do you have any ideas about that?

Hi Dimitri,
(In reply to comment #57)
<snip/>>
> I try to use the trunk 660979 and find a case, when a footnote defined in
> table-body disappears. An example of this is attached. I look for solution
> since some days, but don’t get much with my very modest knowledge of fop. Do
> you have any ideas about that?
This is an oversight. The special code that is executed to handle the forced height set on the table row doesn't handle footnotes. And setting the row height to 33mm is enough to include the line that contains the footnote reference. When the height is set to 32mm that line is handled by the 'normal' code, that knows about footnotes.
I'll see if I can provide a fix in the next days. Thanks for the very simple test case.
Vincent

Hi Vincent,
thank you for the patch. This time another issue with a wrong order of
footnotes. There is a two column table in the attached example, both columns
have footnotes. Sometimes the footnote from the second column precedes the
footnote from the first one. If you delete one block from the first column, the
order will be right.
Dimitri

Hi Dimitri,
(In reply to comment #61)
> Hi Vincent,
>
> thank you for the patch. This time another issue with a wrong order of
> footnotes. There is a two column table in the attached example, both columns
> have footnotes. Sometimes the footnote from the second column precedes the
> footnote from the first one. If you delete one block from the first column, the
> order will be right.
It all depends on what order you should be expecting. If you scan the page in its whole width starting from the top you will find the footnote labeled 2 before the footnote labeled 1. This is basically what FOP is doing.
Of course, it may seem more natural to start from the leftmost column, then go to the following one, etc. But this is particular to that case. With a right-to-left language it will be more natural to start from the rightmost column. Sometimes, the content will be such that the method above will be more natural.
So this is a grey area, and the Recommendation doesn't say anything about that. Your best bet is to re-number the footnotes. Or use something else than footnotes (you may be happy with putting the notes in regular blocks just after the table, for example).
HTH,
Vincent

(In reply to comment #62)
> Hi Dimitri,
>
> (In reply to comment #61)
> > Hi Vincent,
> >
> > thank you for the patch. This time another issue with a wrong order of
> > footnotes. There is a two column table in the attached example, both columns
> > have footnotes. Sometimes the footnote from the second column precedes the
> > footnote from the first one. If you delete one block from the first column, the
> > order will be right.
>
> It all depends on what order you should be expecting. If you scan the page in
> its whole width starting from the top you will find the footnote labeled 2
> before the footnote labeled 1. This is basically what FOP is doing.
>
> Of course, it may seem more natural to start from the leftmost column, then go
> to the following one, etc. But this is particular to that case. With a
> right-to-left language it will be more natural to start from the rightmost
> column. Sometimes, the content will be such that the method above will be more
> natural.
>
> So this is a grey area, and the Recommendation doesn't say anything about that.
> Your best bet is to re-number the footnotes. Or use something else than
> footnotes (you may be happy with putting the notes in regular blocks just after
> the table, for example).
>
> HTH,
> Vincent
Hi Vincent,
I understand your point of view, you are probably right. Anyway, current implementation is not very reliable. If you leave only two blocks in the left column in my example, footnote labeled 1 will be output first, although, according to FOP behavior, we can expect it should be footnote labeled 2.
Dimitri

Hi Dimitri,
(In reply to comment #63)
> (In reply to comment #62)
> > Hi Dimitri,
> >
> > (In reply to comment #61)
> > > Hi Vincent,
> > >
> > > thank you for the patch. This time another issue with a wrong order of
> > > footnotes. There is a two column table in the attached example, both columns
> > > have footnotes. Sometimes the footnote from the second column precedes the
> > > footnote from the first one. If you delete one block from the first column, the
> > > order will be right.
> >
> > It all depends on what order you should be expecting. If you scan the page in
> > its whole width starting from the top you will find the footnote labeled 2
> > before the footnote labeled 1. This is basically what FOP is doing.
> >
> > Of course, it may seem more natural to start from the leftmost column, then go
> > to the following one, etc. But this is particular to that case. With a
> > right-to-left language it will be more natural to start from the rightmost
> > column. Sometimes, the content will be such that the method above will be more
> > natural.
> >
> > So this is a grey area, and the Recommendation doesn't say anything about that.
> > Your best bet is to re-number the footnotes. Or use something else than
> > footnotes (you may be happy with putting the notes in regular blocks just after
> > the table, for example).
> >
> > HTH,
> > Vincent
>
> Hi Vincent,
>
> I understand your point of view, you are probably right. Anyway, current
> implementation is not very reliable. If you leave only two blocks in the left
> column in my example, footnote labeled 1 will be output first, although,
> according to FOP behavior, we can expect it should be footnote labeled 2.
The change I made introduced another bug. It should be fixed now in revision 770635. Sorry about that.
That said, I can think of certain situations involving row-spanning cells where the basic 'rule' stated above does no longer hold. I won't enter the details because they are a bit technical, but interesting issues may arise regarding accessibility, order of reading, etc. (That was a note to self :-) )
Vincent

This is ASF Bugzilla: the Apache Software Foundation bug system. In case
of problems with the functioning of ASF Bugzilla, please contact
bugzilla-admin@apache.org.
Please Note: this e-mail address is only for reporting problems
with ASF Bugzilla. Mail about any other subject will be silently
ignored.