Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/
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/
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/
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