You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Jeremias Maerki <de...@jeremias-maerki.ch> on 2008/06/19 10:01:52 UTC
FOEventHandler method call sequence
I'm coaching a developer implementing an "ODFHandler" for FOP and during
implementation we've stumbled over something: the call sequence of
FOEventHandler methods is not correct. Assume the following FO:
<fo:block>
Text1
<fo:block>
Text2
</fo:block>
</fo:block>
The call sequence at the moment is:
startBlock()
startBlock()
characters(Text1)
characters(Text2)
endBlock()
endBlock()
That's due to flushText() processing in FObjMixed. startBlock() is
effectively called too soon. It would have to be deferred until the
first child is added to the block. Obviously that would apply to all
FObjMixed descendants.
The problem didn't get noticed as AreaTreeHandler doesn't need
start/endBlock() & Co. And RTFHandler does a post-FOTree-buildup
traversal of the FO tree which avoids the problem. OTOH it makes the
start/endBlock methods on FOEventHandler somewhat useless.
My work-around is to split the ODFHandler (and RTFHandler, too) into two
FOEventHandler subclasses. The first just reacts on endPageSequence() to
trigger the FO tree traversal. The second actually contains the logic to
generate ODF/RTF. ATM, almost all methods in RTFHandler begin with:
if (bDefer) {
return;
}
(bDefer is true during FO tree buildup)
Basically, the bDefer variable combines two different behaviours in one
single class which is ugly and makes the code hard to read. This is
better divided into two class which I'll try right away. What I'm not
sure is whether that could be avoided if the call sequence from the FO
tree buildup were fixed (if fixable). The reason for "bDefer":
http://svn.apache.org/viewvc?limit_changes=0&view=rev&revision=197273
Any thoughts anyone?
Thanks,
Jeremias Maerki
Re: FOEventHandler method call sequence
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 19.06.2008 14:49:35 Andreas Delmelle wrote:
> On Jun 19, 2008, at 14:07, Jeremias Maerki wrote:
>
> > On 19.06.2008 10:46:59 Andreas Delmelle wrote:
> >>
> > Thanks for looking into it. Yes, your proposal would solve the problem
> > with the sequence. But FOEventHandler.characters() is still called
> > before white space handling. That would also need to be delayed. I
> > guess
> > I'll have to open a Bugzilla issue and add the test case I've
> > constructed. I don't have enough time to dive any deeper right
> > now. :-(
>
> I see what you mean. In the same sense, there is a gotcha concerning
> fo:character. startCharacter() could be called, while further on, the
> corresponding Character node is removed from the tree... The
> intention was definitely that a FOEventHandler implementation would
> not need to worry about white-space handling, except around formatter-
> generated breaks.
>
> I'll see if I can tackle it. I'm currently working on changing FOText
> to use a java.nio.CharBuffer, and already stumbled upon issues with
> white-space handling, so I may need to revisit that anyway.
That'd be aweseome!!! I got a bit scared by the thought of having to
dive into whitespace handling there. ;-)
> Cheers
>
> Andreas
Jeremias Maerki
Re: FOEventHandler method call sequence
Posted by Andreas Delmelle <an...@telenet.be>.
On Jun 23, 2008, at 11:46, Jeremias Maerki wrote:
Hi Jeremias
> thank you very much for looking into this. This is very helpful!
No problem.
>
> Also, your last comment is probably important for the page breaking
> work,
> too, once FOP gets to the point that it can start layout before a full
> page-sequence has been investigated. It means that it won't be
> possible
> to start layout inside an fo:block, only at its end.
A little sooner in most cases, I think: as soon as /either/ the block
ends, /or/ the next node is known.
In an example like:
<fo:block>
Block Text #1
<fo:inline>
Inline Text #1
</fo:inline>
Block Text #2
<fo:inline>
Inline Text #2
</fo:inline>
Block Text #3
</fo:block>
The FOEventHandler has the guarantee that white-space has been
completely handled for /everything/ preceding the second fo:inline at
the point of its startInline() event. The only thing the handler
needs to take care of is spaces surrounding formatter-generated
linebreaks.
The white-space handling works such that at Block.endOfNode(), we
only have to handle whatever text remains (the portion added between
its last child and the end) and at that point, we also look at the
pending inline(s) [Note: plural only in case of nested fo:inlines].
If there would be no more text between endInline() and endBlock(),
then we know at that point that the trailing space in that text can
also be removed. It is only in that case that the characters() event
is 'unreliable'.
Only for the second inline and the trailing piece "Block Text #3", we
have to wait until endBlock().
So, in theory, for one long fo:block that would span 100 pages, as
long as there are some non-text nodes in between...
Andreas
Re: FOEventHandler method call sequence
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Andreas,
thank you very much for looking into this. This is very helpful!
Also, your last comment is probably important for the page breaking work,
too, once FOP gets to the point that it can start layout before a full
page-sequence has been investigated. It means that it won't be possible
to start layout inside an fo:block, only at its end. For the RTF/ODF
handlers that means the deferral doesn't have to wait until the end of
the page-sequence but only until the end of the current block, if that
helps any.
On 23.06.2008 00:38:22 Andreas Delmelle wrote:
> On Jun 19, 2008, at 14:07, Jeremias Maerki wrote:
>
> > On 19.06.2008 10:46:59 Andreas Delmelle wrote:
> >> On Jun 19, 2008, at 10:01, Jeremias Maerki wrote:
> >>
> >>> I'm coaching a developer implementing an "ODFHandler" for FOP and
> >>> during
> >>> implementation we've stumbled over something: the call sequence of
> >>> FOEventHandler methods is not correct. Assume the following FO:
> >>>
> >>> <fo:block>
> >>> Text1
> >>> <fo:block>
> >>> Text2
> >>> </fo:block>
> >>> </fo:block>
> >>>
> >>> The call sequence at the moment is:
> >>>
> >>> startBlock()
> >>> startBlock()
> >>> characters(Text1)
> >>> characters(Text2)
> >>> endBlock()
> >>> endBlock()
> >>>
> >>> That's due to flushText() processing in FObjMixed. startBlock() is
> >>> effectively called too soon. It would have to be deferred until the
> >>> first child is added to the block. Obviously that would apply to all
> >>> FObjMixed descendants.
> >>
> >> I think this can be avoided in FOTreeBuilder itself.
>
>
> After my recent commits, the assertion attached to Bugzilla #45237
> now passes.
> It's still not entirely correct, but already much better than before.
> At least the FOEventHandler now has the guarantee at end-of-block,
> the white-space removal is complete, and the events are now raised in
> the correct order:
>
> startBlock()
> characters(Text1)
> startBlock()
> characters(Text2)
> endBlock()
> endBlock()
>
> OTOH, I've noticed with our testcase 'inline_white-space-
> treatment_bugzilla45097.xml' that for the following FO:
>
> <fo:block><fo:inline> *** </fo:inline></fo:block>
>
> We still get something like:
>
> startBlock()
> startInline()
> characters(*** )
> endInline()
> endBlock()
>
> It is only at the end of the block that the XMLWhiteSpaceHandler has
> enough context to decide whether the trailing white-space in the
> inline is ignorable; if some inline would follow, at least one space
> should be preserved. Some sort of deferral mechanism is still needed
> in the FOEventHandler implementations. The corresponding FOText
> descendant of the Inline will only be correct as soon as endBlock()
> has passed, or a following node has been added to the block...
>
>
> Cheers
>
> Andreas
Jeremias Maerki
Re: FOEventHandler method call sequence
Posted by Andreas Delmelle <an...@telenet.be>.
On Jun 19, 2008, at 14:07, Jeremias Maerki wrote:
> On 19.06.2008 10:46:59 Andreas Delmelle wrote:
>>
> Thanks for looking into it. Yes, your proposal would solve the problem
> with the sequence. But FOEventHandler.characters() is still called
> before white space handling. That would also need to be delayed. I
> guess
> I'll have to open a Bugzilla issue and add the test case I've
> constructed. I don't have enough time to dive any deeper right
> now. :-(
I see what you mean. In the same sense, there is a gotcha concerning
fo:character. startCharacter() could be called, while further on, the
corresponding Character node is removed from the tree... The
intention was definitely that a FOEventHandler implementation would
not need to worry about white-space handling, except around formatter-
generated breaks.
I'll see if I can tackle it. I'm currently working on changing FOText
to use a java.nio.CharBuffer, and already stumbled upon issues with
white-space handling, so I may need to revisit that anyway.
Cheers
Andreas
Re: FOEventHandler method call sequence
Posted by Andreas Delmelle <an...@telenet.be>.
On Jun 19, 2008, at 14:07, Jeremias Maerki wrote:
> On 19.06.2008 10:46:59 Andreas Delmelle wrote:
>> On Jun 19, 2008, at 10:01, Jeremias Maerki wrote:
>>
>>> I'm coaching a developer implementing an "ODFHandler" for FOP and
>>> during
>>> implementation we've stumbled over something: the call sequence of
>>> FOEventHandler methods is not correct. Assume the following FO:
>>>
>>> <fo:block>
>>> Text1
>>> <fo:block>
>>> Text2
>>> </fo:block>
>>> </fo:block>
>>>
>>> The call sequence at the moment is:
>>>
>>> startBlock()
>>> startBlock()
>>> characters(Text1)
>>> characters(Text2)
>>> endBlock()
>>> endBlock()
>>>
>>> That's due to flushText() processing in FObjMixed. startBlock() is
>>> effectively called too soon. It would have to be deferred until the
>>> first child is added to the block. Obviously that would apply to all
>>> FObjMixed descendants.
>>
>> I think this can be avoided in FOTreeBuilder itself.
After my recent commits, the assertion attached to Bugzilla #45237
now passes.
It's still not entirely correct, but already much better than before.
At least the FOEventHandler now has the guarantee at end-of-block,
the white-space removal is complete, and the events are now raised in
the correct order:
startBlock()
characters(Text1)
startBlock()
characters(Text2)
endBlock()
endBlock()
OTOH, I've noticed with our testcase 'inline_white-space-
treatment_bugzilla45097.xml' that for the following FO:
<fo:block><fo:inline> *** </fo:inline></fo:block>
We still get something like:
startBlock()
startInline()
characters(*** )
endInline()
endBlock()
It is only at the end of the block that the XMLWhiteSpaceHandler has
enough context to decide whether the trailing white-space in the
inline is ignorable; if some inline would follow, at least one space
should be preserved. Some sort of deferral mechanism is still needed
in the FOEventHandler implementations. The corresponding FOText
descendant of the Inline will only be correct as soon as endBlock()
has passed, or a following node has been added to the block...
Cheers
Andreas
Re: FOEventHandler method call sequence
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 19.06.2008 10:46:59 Andreas Delmelle wrote:
> On Jun 19, 2008, at 10:01, Jeremias Maerki wrote:
>
> > I'm coaching a developer implementing an "ODFHandler" for FOP and
> > during
> > implementation we've stumbled over something: the call sequence of
> > FOEventHandler methods is not correct. Assume the following FO:
> >
> > <fo:block>
> > Text1
> > <fo:block>
> > Text2
> > </fo:block>
> > </fo:block>
> >
> > The call sequence at the moment is:
> >
> > startBlock()
> > startBlock()
> > characters(Text1)
> > characters(Text2)
> > endBlock()
> > endBlock()
> >
> > That's due to flushText() processing in FObjMixed. startBlock() is
> > effectively called too soon. It would have to be deferred until the
> > first child is added to the block. Obviously that would apply to all
> > FObjMixed descendants.
>
> I think this can be avoided in FOTreeBuilder itself. I seem to
> remember having changed this locally at one point (but apparently
> I've reverted this, even though, IIRC, it didn't have any noticeable
> impact for the rest...)
>
> Currently in MainFOHandler.startElement() we do, roughly:
>
> 1° newNode = FOMaker.make(parentNode);
> 2° newNode.startOfNode();
> 3° parentNode.addChildNode(newNode);
> 4° newNode = parentNode;
>
> Step 2 and 3 should, IMO be swapped. startOfNode() should be called
> after the new node has been added as a child, rather than before.
>
> This should fix the problem, as flushText() for 'Text1' will then be
> called before the inner block's startBlock(), but I haven't recently
> tested if this breaks anything else...
Thanks for looking into it. Yes, your proposal would solve the problem
with the sequence. But FOEventHandler.characters() is still called
before white space handling. That would also need to be delayed. I guess
I'll have to open a Bugzilla issue and add the test case I've
constructed. I don't have enough time to dive any deeper right now. :-(
>
> HTH!
>
> Andreas
Jeremias Maerki
Re: FOEventHandler method call sequence
Posted by Andreas Delmelle <an...@telenet.be>.
On Jun 19, 2008, at 10:01, Jeremias Maerki wrote:
> I'm coaching a developer implementing an "ODFHandler" for FOP and
> during
> implementation we've stumbled over something: the call sequence of
> FOEventHandler methods is not correct. Assume the following FO:
>
> <fo:block>
> Text1
> <fo:block>
> Text2
> </fo:block>
> </fo:block>
>
> The call sequence at the moment is:
>
> startBlock()
> startBlock()
> characters(Text1)
> characters(Text2)
> endBlock()
> endBlock()
>
> That's due to flushText() processing in FObjMixed. startBlock() is
> effectively called too soon. It would have to be deferred until the
> first child is added to the block. Obviously that would apply to all
> FObjMixed descendants.
I think this can be avoided in FOTreeBuilder itself. I seem to
remember having changed this locally at one point (but apparently
I've reverted this, even though, IIRC, it didn't have any noticeable
impact for the rest...)
Currently in MainFOHandler.startElement() we do, roughly:
1° newNode = FOMaker.make(parentNode);
2° newNode.startOfNode();
3° parentNode.addChildNode(newNode);
4° newNode = parentNode;
Step 2 and 3 should, IMO be swapped. startOfNode() should be called
after the new node has been added as a child, rather than before.
This should fix the problem, as flushText() for 'Text1' will then be
called before the inner block's startBlock(), but I haven't recently
tested if this breaks anything else...
HTH!
Andreas