Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

2009-06-23 Thread Andreas Delmelle

On 23 Jun 2009, at 19:11, Vincent Hennebert wrote:

Hi Vincent



Maybe nothing, since the check will always return false (should be
caught during FO tree validation, but then there's relaxed  
validation...).


o.a.fop.fo.pagination.Flow explicitly checks that every child  
element is

a block-level element, even in relaxed validation mode. Also, after
replacing the log with a ‘throw new IllegalStateException()’ the whole
test suite ran without any problem. So I considered it safe to remove.


OK. The only thing I remembered about the check, was modifying it when  
implementing fo:wrappers as direct children of the fo:flow, but as I  
read it now, it seems like that could indeed only happen if the FO is  
invalid, and so the check is obsolete.





All the other changes receive my blessing! :-)


Thanks for double-checking!


Also, no problem! I have a lot more of those cleanups following  
shortly (removal of superfluous code, renaming stray Hungarians...),  
in the classes affected by the column-keeps patch. AFAICT from your  
commits, the conflicts should be minimal, if any.


Just mentioning this so you're aware. If you find anything dubious,  
chances are that I've already changed it locally, and it will be  
committed to the trunk pretty soon.



Regards,

Andreas




Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

2009-06-23 Thread Vincent Hennebert
Hi Andreas,

Andreas Delmelle wrote:
> On 23 Jun 2009, at 17:50, vhennebert apache org wrote:
> 
>> Author: vhennebert
>> Date: Tue Jun 23 15:50:15 2009
>> New Revision: 787733
>>
>> URL: http://svn.apache.org/viewvc?rev=787733&view=rev
>> Log:
>> Code clean-up
>>
> 
>> http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
> 
>> -if (!(curLM instanceof WrapperLayoutManager)
>> -&& curLM instanceof InlineLevelLayoutManager) {
>> -log.error("inline area not allowed under flow -
>> ignoring");
>> -curLM.setFinished(true);
>> -continue;
>> -}
> 
> This may be too much cleanup. I'm not entirely certain, but the
> 'continue' statement is meant to prevent this condition from crashing
> FOP on something we can perfectly recover from... (IIRC, without this
> check, we would end up with a ClassCastException when adding the areas)
> 
> Admitted, this event would better be routed through the event mechanism,
> so users can decide for themselves. As long as that has not been done...
> 
> Maybe nothing, since the check will always return false (should be
> caught during FO tree validation, but then there's relaxed validation...).

o.a.fop.fo.pagination.Flow explicitly checks that every child element is
a block-level element, even in relaxed validation mode. Also, after
replacing the log with a ‘throw new IllegalStateException()’ the whole
test suite ran without any problem. So I considered it safe to remove.


> All the other changes receive my blessing! :-)

Thanks for double-checking!

Vincent


Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

2009-06-23 Thread Andreas Delmelle

On 23 Jun 2009, at 17:50, vhenneb...@apache.org wrote:


Author: vhennebert
Date: Tue Jun 23 15:50:15 2009
New Revision: 787733

URL: http://svn.apache.org/viewvc?rev=787733&view=rev
Log:
Code clean-up




http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
=
=
=
=
=
=
=
=
==



-if (!(curLM instanceof WrapperLayoutManager)
-&& curLM instanceof InlineLevelLayoutManager) {
-log.error("inline area not allowed under flow -  
ignoring");

-curLM.setFinished(true);
-continue;
-}


This may be too much cleanup. I'm not entirely certain, but the  
'continue' statement is meant to prevent this condition from crashing  
FOP on something we can perfectly recover from... (IIRC, without this  
check, we would end up with a ClassCastException when adding the areas)


Admitted, this event would better be routed through the event  
mechanism, so users can decide for themselves. As long as that has not  
been done...


Maybe nothing, since the check will always return false (should be  
caught during FO tree validation, but then there's relaxed  
validation...).


All the other changes receive my blessing! :-)


Thanks!

Regards

Andreas