--- Comment #38 from Andreas L. Delmelle <[EMAIL PROTECTED]>  2008-05-16 
11:29:36 PST ---
(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.

Configure bugmail:
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to