> Author: jeremias
> Date: Wed Jun 18 02:02:45 2008
> New Revision: 669118
> URL: http://svn.apache.org/viewvc?rev=669118&view=rev
> Log:
> Bugzilla #44412:
> Regression fix for empty pages caused by multiple collapsible breaks.
> No more empty block areas if a break-before occurs on the first child of an 
> FO to match the behaviour of tables and other FO implementations 
> (clarification by XSL WG pending).
> Added an accessor interface for break-before/-after to avoid long if..else 
> lists in BlockStackingLayoutManager.

I’m afraid I’m not happy with this change. There are a couple of issues
that should be looked at IMO.

Simple things first:
- BlockStackingLM.getBreakBefore(boolean) is always given true as its
  parameter. Why specifying a parameter then? Plus this method is called
  only within BlockStackingLM, so it should be made private (at least
  for now).
- BlockStackingLM.getBreakBefore(): same here, this method should be
  made private. And if fobj doesn’t implement BreakPropertySet, why
  should EN_AUTO be returned? If this method is called on an object that
  doesn’t support it, I guess it should rather throw an exception than
  silently return a default value. Otherwise that makes it difficult to
  track down errors.
- why is childLMForBreakSkip a weak reference?? What will happen if the
  object has been released at the moment where it’s retrieved? Will the
  code still behave correctly? Furthermore, this childLMForBreakSkip
  object will by definition always be the first child LM of the current
  LM. So this field might not be needed at all in the end.
- I was hoping that the way I implemented breaks in table could be
  generalized to the other LMs. Maybe that approach has its flaws that
  I haven’t noticed but so far this works for tables.
  Basically the idea is that instead of creating break elements, a LM
  sets a flag on the LayoutContext it was given through the
  getNextKnuthElements call. Then it’s the ancestor block stacking LM
  that decides whether a break element must be produced (when the child
  LM is in the middle of the sequence) or if the break property must be
  passed on to the parent LM (when the child LM is the first child
  of the block stacking LM).
  See TableContentLM.getKnuthElementsForRowIterator,

I think it would be very unfortunate to have two different mechanisms
for handling breaks. The thing is, I’ll be offline for the next 10 days
so I’ll have no time to do/review anything, and I realise that we
wouldn’t want to delay the release of 0.95 too much. We could imagine to
leave this change as is in the 0.95 branch, but it should not be merged
back into the Trunk.


Vincent Hennebert                            Anyware Technologies
http://people.apache.org/~vhennebert         http://www.anyware-tech.com
Apache FOP Committer                         FOP Development/Consulting

Reply via email to