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 2006/04/07 10:06:45 UTC
Fixing leading whitespace on a line (was: How about a release?)
On 06.04.2006 20:50:18 Peter S. Housel wrote:
> > WDYT?
>
> >From my perspective as a user, I think it's essential to fix the problem
> with preserving whitespace after a newline (for <pre>-style output) before
> doing another release.
>
> -Peter-
Yeah, that's an annoying problem. A quick fix may not be possible,
however. The following patch is simple and does its job for most cases.
It simply makes sure that leading spaces are not ignored anymore by the
line breaker. However, this has a side-effect which breaks a few test
cases, namely:
- inline_block_nested_4
- inline_border_padding_block_nested_1
- inline_border_padding_block_nested_2
These test are about blocks nested in inlines. Here, a linefeed is
converted to a space (by the XMLWhiteSpaceHandler) which ends up as a
leading space on the line right after the nested block. Up to date, this
leading space was suppressed by the Knuth algorithm (leading glues and
penalties are ignored). I think the behaviour after applying the patch
below could actually be correct but may be unexpected from a user
perspective. But linefeed-treatment is on the default ("treat-as-space")
in these test cases.
However, if I set linefeed-treatment to "ignore", the leading space
still remains. Only if I set it to "treat-as-zero-width-space" does the
leading space vanish.
Any insight from the whitespace specialists? I'm not sure about it and
don't want to apply that "quick fix" without a second opinion.
Index: TextLayoutManager.java
===================================================================
--- TextLayoutManager.java (Revision 391740)
+++ TextLayoutManager.java (Arbeitskopie)
@@ -539,6 +539,10 @@
vecAreaInfo.add(ai);
// create the elements
+ if (sequence.size() == 0) {
+ sequence.add(new KnuthInlineBox(0, null,
+ notifyPos(new LeafPosition(this, -1)), true));
+ }
sequence.addAll
(createElementsForASpace(alignment, ai, vecAreaInfo.size() - 1));
Jeremias Maerki
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Simon Pepping <sp...@leverkruid.nl>.
On Sun, Apr 09, 2006 at 02:55:05PM +0200, Jeremias Maerki wrote:
>
> On 09.04.2006 14:32:39 Manuel Mall wrote:
> > it comes to Knuth element generation. But if we move the element list
> > generation to LineLM level we break the generic LM interface (may be
> > its OK if inline type LMs expose a different interface than block level
> > LMs?). Also if we let the LineLM build the Knuth element list it must
>
> I don't think it's a problem if inline LMs expose a different interface.
> We're definitely not bound to keeping everything as it is, if this
> offers chance to do things right.
I agree that there is no a priori reason for the InlineLMs and
BlockLevelLMs to expose the same interface.
Simon
--
Simon Pepping
home page: http://www.leverkruid.nl
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 09.04.2006 14:32:39 Manuel Mall wrote:
> On Sunday 09 April 2006 20:13, Jeremias Maerki wrote:
> > On 08.04.2006 10:29:33 Manuel Mall wrote:
> > > On Friday 07 April 2006 23:13, Manuel Mall wrote:
> > > > On Friday 07 April 2006 19:14, Manuel Mall wrote:
> > > > > On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> > > > > > On 06.04.2006 20:50:18 Peter S. Housel wrote:
> > > >
> <snip/>
> > > However, I am not sure if the spaces 'embedded' in the
> > > <fo:character character=" " /> elements are dealt with correctly.
> >
> > No, they are not. :-( It seems we don't have the ability to generated
> > adjustable spaces from fo:character. This got me thinking about how
> > to improve this: I think we should investigate a way that would allow
> > us to fold the characters from fo:character elements into the main
> > text array of an FOText. Maybe a flyweight pattern approach could
> > help specifying the applicable properties for each character. That
> > means that the character borders and such should be dealt with in
> > TextLM (CharLM would go). The code might get a little more
> > complicated but it would solve the problem above and would address
> > the fact that all characters should be internally converted to
> > fo:character objects as the spec suggests. Only we'd be doing this
> > the other way around. Not sure. Maybe there's also an easier way by
> > enhancing CharLM. WDYT?
> >
>
> I think pulling it up to the TextLM is a step into the right direction
> but as I said before I still do believe it needs to go up to the LineLM
> level as character handling (white space handling, break decisions,
> determine word boundaries for hyphenation, ...) spans inline
> boundaries. Therefore a pattern like a character iterator to be exposed
> by the inline LMs which would allow the LineLM to iterate through all
> characters (with their properties) it has to deal with appears to me
> more suitable. While this is all doable without too much difficulties
> (although blocks in inlines, leaders, borders, and the like do pose
> some challenges to this pattern) the decision if one would need to move
> the Knuth element list generation to LineLM level as well is IMO the
> critical one here. If we leave the Knuth element list generation at
> inline LM level we have again problems with stuff crossing boundaries
> and also need a means of signalling back to the LM who provided the
> character through the above mentioned iterator what to do with it when
> it comes to Knuth element generation. But if we move the element list
> generation to LineLM level we break the generic LM interface (may be
> its OK if inline type LMs expose a different interface than block level
> LMs?). Also if we let the LineLM build the Knuth element list it must
> be provided with enough information (or suitable interfaces) to be able
> to construct the "area info" objects required by the LMs to actually
> build the areas during the add area phase.
>
> Decisions, decisions, ...
Yes, yes. But that's the fun about it, right? :-) I didn't think about
the inline spanning when I wrote that. You've definitely got the better
"big picture" and your proposed direction certainly makes sense to me.
I don't think it's a problem if inline LMs expose a different interface.
We're definitely not bound to keeping everything as it is, if this
offers chance to do things right.
Jeremias Maerki
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Manuel Mall <mm...@arcus.com.au>.
On Sunday 09 April 2006 20:13, Jeremias Maerki wrote:
> On 08.04.2006 10:29:33 Manuel Mall wrote:
> > On Friday 07 April 2006 23:13, Manuel Mall wrote:
> > > On Friday 07 April 2006 19:14, Manuel Mall wrote:
> > > > On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> > > > > On 06.04.2006 20:50:18 Peter S. Housel wrote:
> > >
<snip/>
> > However, I am not sure if the spaces 'embedded' in the
> > <fo:character character=" " /> elements are dealt with correctly.
>
> No, they are not. :-( It seems we don't have the ability to generated
> adjustable spaces from fo:character. This got me thinking about how
> to improve this: I think we should investigate a way that would allow
> us to fold the characters from fo:character elements into the main
> text array of an FOText. Maybe a flyweight pattern approach could
> help specifying the applicable properties for each character. That
> means that the character borders and such should be dealt with in
> TextLM (CharLM would go). The code might get a little more
> complicated but it would solve the problem above and would address
> the fact that all characters should be internally converted to
> fo:character objects as the spec suggests. Only we'd be doing this
> the other way around. Not sure. Maybe there's also an easier way by
> enhancing CharLM. WDYT?
>
I think pulling it up to the TextLM is a step into the right direction
but as I said before I still do believe it needs to go up to the LineLM
level as character handling (white space handling, break decisions,
determine word boundaries for hyphenation, ...) spans inline
boundaries. Therefore a pattern like a character iterator to be exposed
by the inline LMs which would allow the LineLM to iterate through all
characters (with their properties) it has to deal with appears to me
more suitable. While this is all doable without too much difficulties
(although blocks in inlines, leaders, borders, and the like do pose
some challenges to this pattern) the decision if one would need to move
the Knuth element list generation to LineLM level as well is IMO the
critical one here. If we leave the Knuth element list generation at
inline LM level we have again problems with stuff crossing boundaries
and also need a means of signalling back to the LM who provided the
character through the above mentioned iterator what to do with it when
it comes to Knuth element generation. But if we move the element list
generation to LineLM level we break the generic LM interface (may be
its OK if inline type LMs expose a different interface than block level
LMs?). Also if we let the LineLM build the Knuth element list it must
be provided with enough information (or suitable interfaces) to be able
to construct the "area info" objects required by the LMs to actually
build the areas during the add area phase.
Decisions, decisions, ...
>
> Jeremias Maerki
Manuel
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 08.04.2006 10:29:33 Manuel Mall wrote:
> On Friday 07 April 2006 23:13, Manuel Mall wrote:
> > On Friday 07 April 2006 19:14, Manuel Mall wrote:
> > > On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> > > > On 06.04.2006 20:50:18 Peter S. Housel wrote:
> >
> > <snip/>
> >
> Committed my changes.
Looks good! Thanks a lot!
> As far as I can tell the various text-align property values work
> correctly even if white space is not collapsed. That is assuming we
> have something like:
> word1<sp><sp><sp>word2<sp><sp>word3
> and do not collapse whitespace but specify text-align="justify" each
> <sp> is treated as a separate word space.
>
> I think the disabled test case block_white-space-collapse_2.xml is also
> fixed. At least the failure reason given in disabled_testcases.xml is
> addressed now.
Yes. I'm a little confused about the text in there. Anyway, the
behaviour seems correct as are the generated element lists.
I think I'm going to enhance the test a little and add a new one that
tests a few specialities. I've found a bug, too: If I use
text-align-last="justify" the nbsp characters are not adjustable. I've
locally added nbsp to the list of adjustable characters in CharUtilities.
I'll upload as soon as I've got the checks ready.
> However, I am not sure if the spaces 'embedded' in the
> <fo:character character=" " /> elements are dealt with correctly.
No, they are not. :-( It seems we don't have the ability to generated
adjustable spaces from fo:character. This got me thinking about how to
improve this: I think we should investigate a way that would allow us to
fold the characters from fo:character elements into the main text array
of an FOText. Maybe a flyweight pattern approach could help specifying
the applicable properties for each character. That means that the
character borders and such should be dealt with in TextLM (CharLM would
go). The code might get a little more complicated but it would solve the
problem above and would address the fact that all characters should be
internally converted to fo:character objects as the spec suggests. Only
we'd be doing this the other way around. Not sure. Maybe there's also an
easier way by enhancing CharLM. WDYT?
> Therefore I haven't removed that testcase from the disabled list. A
> similar comment applies to block_linefeed-treatment.xml. The comment in
> disabled_testcases.xml that the testcase fails because of incorrect
> handling of ZWSP is not correct any more. However, the <fo:character
> issue probably still remains. May be someone else can cast their eyes
> over these two cases and their current behaviour?
<snip/>
> > PS: Good to be back working on FOP - really missed it for the last
> > couple of months.
I'm glad to hear that and to have you back.
Jeremias Maerki
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Manuel Mall <mm...@arcus.com.au>.
On Friday 07 April 2006 23:13, Manuel Mall wrote:
> On Friday 07 April 2006 19:14, Manuel Mall wrote:
> > On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> > > On 06.04.2006 20:50:18 Peter S. Housel wrote:
>
> <snip/>
>
Committed my changes.
As far as I can tell the various text-align property values work
correctly even if white space is not collapsed. That is assuming we
have something like:
word1<sp><sp><sp>word2<sp><sp>word3
and do not collapse whitespace but specify text-align="justify" each
<sp> is treated as a separate word space.
I think the disabled test case block_white-space-collapse_2.xml is also
fixed. At least the failure reason given in disabled_testcases.xml is
addressed now. However, I am not sure if the spaces 'embedded' in the
<fo:character character=" " /> elements are dealt with correctly.
Therefore I haven't removed that testcase from the disabled list. A
similar comment applies to block_linefeed-treatment.xml. The comment in
disabled_testcases.xml that the testcase fails because of incorrect
handling of ZWSP is not correct any more. However, the <fo:character
issue probably still remains. May be someone else can cast their eyes
over these two cases and their current behaviour?
>
> > > Jeremias Maerki
> >
> > Manuel
>
> Manuel
>
> PS: Good to be back working on FOP - really missed it for the last
> couple of months.
Manuel
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Manuel Mall <mm...@arcus.com.au>.
On Friday 07 April 2006 19:14, Manuel Mall wrote:
> On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> > On 06.04.2006 20:50:18 Peter S. Housel wrote:
<snip/>
> >
> > Any insight from the whitespace specialists? I'm not sure about it
> > and don't want to apply that "quick fix" without a second opinion.
>
> I have some time this weekend and will look into this in a bit more
> detail. Shouldn't be too hard to find a "semi-good" fix for this bug.
>
Just a quick progress report.
I have what I think is a working patch. All existing tests pass and a
new test for white-space="pre" seems to produce the correct results.
Need to do some more testing for some "unusual" property combinations
like: white-space-treatment="preserve" and text-align="justify" or
"center" although I am not 100% certain what outcomes to expect in some
of these cases.
And while I am at it I want to look at the other values for the
white-space-treatment property (ignore-if-before/after-linefeed).
It also transpired that white-space-treatment="preserve" is sort of the
same as replacing <SPACE>... with <NBSP><ZWSP>.... This means the Knuth
sequences I generate for the white-space-treatment="preserve" case
should also be able to be used for zero-width spaces. So I'll look into
that as well.
> >
> > Jeremias Maerki
>
> Manuel
Manuel
PS: Good to be back working on FOP - really missed it for the last
couple of months.
Re: Fixing leading whitespace on a line (was: How about a release?)
Posted by Manuel Mall <mm...@arcus.com.au>.
On Friday 07 April 2006 16:06, Jeremias Maerki wrote:
> On 06.04.2006 20:50:18 Peter S. Housel wrote:
> > > WDYT?
> > >
> > >From my perspective as a user, I think it's essential to fix the
> > > problem
> >
> > with preserving whitespace after a newline (for <pre>-style output)
> > before doing another release.
> >
> > -Peter-
>
> Yeah, that's an annoying problem. A quick fix may not be possible,
> however. The following patch is simple and does its job for most
> cases. It simply makes sure that leading spaces are not ignored
> anymore by the line breaker. However, this has a side-effect which
> breaks a few test cases, namely:
> - inline_block_nested_4
> - inline_border_padding_block_nested_1
> - inline_border_padding_block_nested_2
>
> These test are about blocks nested in inlines. Here, a linefeed is
> converted to a space (by the XMLWhiteSpaceHandler) which ends up as a
> leading space on the line right after the nested block. Up to date,
> this leading space was suppressed by the Knuth algorithm (leading
> glues and penalties are ignored). I think the behaviour after
> applying the patch below could actually be correct but may be
> unexpected from a user perspective. But linefeed-treatment is on the
> default ("treat-as-space") in these test cases.
>
> However, if I set linefeed-treatment to "ignore", the leading space
> still remains. Only if I set it to "treat-as-zero-width-space" does
> the leading space vanish.
>
> Any insight from the whitespace specialists? I'm not sure about it
> and don't want to apply that "quick fix" without a second opinion.
I have some time this weekend and will look into this in a bit more
detail. Shouldn't be too hard to find a "semi-good" fix for this bug.
>
> Index: TextLayoutManager.java
> ===================================================================
> --- TextLayoutManager.java (Revision 391740)
> +++ TextLayoutManager.java (Arbeitskopie)
> @@ -539,6 +539,10 @@
> vecAreaInfo.add(ai);
>
> // create the elements
> + if (sequence.size() == 0) {
> + sequence.add(new KnuthInlineBox(0, null,
> + notifyPos(new LeafPosition(this, -1)),
> true)); + }
> sequence.addAll
> (createElementsForASpace(alignment, ai,
> vecAreaInfo.size() - 1));
>
> Jeremias Maerki
Manuel