https://issues.apache.org/bugzilla/show_bug.cgi?id=37579
--- Comment #37 from Vincent Hennebert <[EMAIL PROTECTED]> 2008-05-16 10:16:00 PST --- (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 -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
