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/03/19 15:49:23 UTC

Re: svn commit: r635686 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/

Vincent,

why did you disable this warning here? What's there to clarify? I
stumbled over this when trying to merge the latest changes from trunk
into the feedback branch.

On 10.03.2008 21:52:48 vhennebert wrote:
> Author: vhennebert
> Date: Mon Mar 10 13:52:43 2008
> New Revision: 635686
> 
> URL: http://svn.apache.org/viewvc?rev=635686&view=rev
> Log:
> Bugfix: forced break ignored when the minimum height of a table-row isn't reached
> 
<snip/>
RowGroupLayoutManager:

>              row.setHeight(rowHeights[rgi]);
> -            row.setExplicitHeight(explicitRowHeights[rgi]);
> -            if (maxCellBPD > row.getExplicitHeight().max) {
> -                log.warn(FONode.decorateWithContextInfo(
> -                        "The contents of row " + (row.getIndex() + 1) 
> -                        + " are taller than they should be (there is a"
> -                        + " block-progression-dimension or height constraint on the indicated row)."
> -                        + " Due to its contents the row grows"
> -                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
> -                        + " any taller than " + row.getExplicitHeight() + " millipoints.", 
> -                        row.getTableRow()));
> -            }
> -        }
> -        if (log.isDebugEnabled()) {
> -            log.debug("rowGroup:");
> -            for (int i = 0; i < rowHeights.length; i++) {
> -                log.debug("  height=" + rowHeights[i] + " explicit=" + explicitRowHeights[i]);
> +            row.setExplicitHeight(explicitRowHeight);
> +            // TODO re-enable and improve after clarification
> +//            if (maxCellBPD > row.getExplicitHeight().max) {
> +//                log.warn(FONode.decorateWithContextInfo(
> +//                        "The contents of row " + (row.getIndex() + 1) 
> +//                        + " are taller than they should be (there is a"
> +//                        + " block-progression-dimension or height constraint
> +//                        + " on the indicated row)."
> +//                        + " Due to its contents the row grows"
> +//                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
> +//                        + " any taller than " + row.getExplicitHeight() + " millipoints.", 
> +//                        row.getTableRow()));
> +//            }
> +            if (log.isDebugEnabled()) {
> +                log.debug("  height=" + rowHeights[rgi] + " explicit=" + explicitRowHeight);
>              }
>          }
<snip/>


Jeremias Maerki


Re: svn commit: r635686 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Thanks, Vincent. I didn't remember that one. It's probably a bit
dangerous to rely on things in CSS2 since these references will all be
removed for XSL 2.0. Still, you are right that things are not 100% clear
perhaps. I'll add a todo in the feedback branch so an event is added
once this can be reenabled. At any rate, I consider the check/event
useful as you sometimes have to work with fixed heights and you want to
notice if the content doesn't fit.

On 19.03.2008 17:53:34 Vincent Hennebert wrote:
> Hi Jeremias,
> 
> Jeremias Maerki wrote:
> > Vincent,
> >
> > why did you disable this warning here? What's there to clarify? I
> > stumbled over this when trying to merge the latest changes from trunk
> > into the feedback branch.
> 
> I had to make some changes in the way row heights are computed, in order
> to tackle the forced break vs fixed row height issue (rev. 635686). This
> broke the test that is performed to issue the warning, and given that
> it’s not clear yet whether cell borders and border-separation must play
> or not in the fixed height [1] I thought it was not worth trying to
> re-enable that just now.
> 
> Moreover, CSS2 explicitly states that the final height must be the
> maximum of the height specified on the row/cells and the content height,
> so I’m not even sure a warning is to be expected here. While an
> info-level message would certainly make sense, that made me even less
> reluctant to leave it commented out for now.
> 
> [1] http://mail-archives.apache.org/mod_mbox/xmlgraphics-fop-dev/200801.mbox/%3c479DFFBA.8020500@anyware-tech.com%3e
> 
> Vincent
> 
> 
> > On 10.03.2008 21:52:48 vhennebert wrote:
> >> Author: vhennebert
> >> Date: Mon Mar 10 13:52:43 2008
> >> New Revision: 635686
> >>
> >> URL: http://svn.apache.org/viewvc?rev=635686&view=rev
> >> Log:
> >> Bugfix: forced break ignored when the minimum height of a table-row isn't reached
> >>
> > <snip/>
> > RowGroupLayoutManager:
> >
> >>              row.setHeight(rowHeights[rgi]);
> >> -            row.setExplicitHeight(explicitRowHeights[rgi]);
> >> -            if (maxCellBPD > row.getExplicitHeight().max) {
> >> -                log.warn(FONode.decorateWithContextInfo(
> >> -                        "The contents of row " + (row.getIndex() + 1)
> >> -                        + " are taller than they should be (there is a"
> >> -                        + " block-progression-dimension or height constraint on the indicated row)."
> >> -                        + " Due to its contents the row grows"
> >> -                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
> >> -                        + " any taller than " + row.getExplicitHeight() + " millipoints.",
> >> -                        row.getTableRow()));
> >> -            }
> >> -        }
> >> -        if (log.isDebugEnabled()) {
> >> -            log.debug("rowGroup:");
> >> -            for (int i = 0; i < rowHeights.length; i++) {
> >> -                log.debug("  height=" + rowHeights[i] + " explicit=" + explicitRowHeights[i]);
> >> +            row.setExplicitHeight(explicitRowHeight);
> >> +            // TODO re-enable and improve after clarification
> >> +//            if (maxCellBPD > row.getExplicitHeight().max) {
> >> +//                log.warn(FONode.decorateWithContextInfo(
> >> +//                        "The contents of row " + (row.getIndex() + 1)
> >> +//                        + " are taller than they should be (there is a"
> >> +//                        + " block-progression-dimension or height constraint
> >> +//                        + " on the indicated row)."
> >> +//                        + " Due to its contents the row grows"
> >> +//                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
> >> +//                        + " any taller than " + row.getExplicitHeight() + " millipoints.",
> >> +//                        row.getTableRow()));
> >> +//            }
> >> +            if (log.isDebugEnabled()) {
> >> +                log.debug("  height=" + rowHeights[rgi] + " explicit=" + explicitRowHeight);
> >>              }
> >>          }
> > <snip/>
> >
> >
> > Jeremias Maerki
> >
> 
> -- 
> Vincent Hennebert                            Anyware Technologies
> http://people.apache.org/~vhennebert         http://www.anyware-tech.com
> Apache FOP Committer                         FOP Development/Consulting




Jeremias Maerki


Re: svn commit: r635686 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/

Posted by Vincent Hennebert <vi...@anyware-tech.com>.
Hi Jeremias,

Jeremias Maerki wrote:
> Vincent,
>
> why did you disable this warning here? What's there to clarify? I
> stumbled over this when trying to merge the latest changes from trunk
> into the feedback branch.

I had to make some changes in the way row heights are computed, in order
to tackle the forced break vs fixed row height issue (rev. 635686). This
broke the test that is performed to issue the warning, and given that
it’s not clear yet whether cell borders and border-separation must play
or not in the fixed height [1] I thought it was not worth trying to
re-enable that just now.

Moreover, CSS2 explicitly states that the final height must be the
maximum of the height specified on the row/cells and the content height,
so I’m not even sure a warning is to be expected here. While an
info-level message would certainly make sense, that made me even less
reluctant to leave it commented out for now.

[1] http://mail-archives.apache.org/mod_mbox/xmlgraphics-fop-dev/200801.mbox/%3c479DFFBA.8020500@anyware-tech.com%3e

Vincent


> On 10.03.2008 21:52:48 vhennebert wrote:
>> Author: vhennebert
>> Date: Mon Mar 10 13:52:43 2008
>> New Revision: 635686
>>
>> URL: http://svn.apache.org/viewvc?rev=635686&view=rev
>> Log:
>> Bugfix: forced break ignored when the minimum height of a table-row isn't reached
>>
> <snip/>
> RowGroupLayoutManager:
>
>>              row.setHeight(rowHeights[rgi]);
>> -            row.setExplicitHeight(explicitRowHeights[rgi]);
>> -            if (maxCellBPD > row.getExplicitHeight().max) {
>> -                log.warn(FONode.decorateWithContextInfo(
>> -                        "The contents of row " + (row.getIndex() + 1)
>> -                        + " are taller than they should be (there is a"
>> -                        + " block-progression-dimension or height constraint on the indicated row)."
>> -                        + " Due to its contents the row grows"
>> -                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
>> -                        + " any taller than " + row.getExplicitHeight() + " millipoints.",
>> -                        row.getTableRow()));
>> -            }
>> -        }
>> -        if (log.isDebugEnabled()) {
>> -            log.debug("rowGroup:");
>> -            for (int i = 0; i < rowHeights.length; i++) {
>> -                log.debug("  height=" + rowHeights[i] + " explicit=" + explicitRowHeights[i]);
>> +            row.setExplicitHeight(explicitRowHeight);
>> +            // TODO re-enable and improve after clarification
>> +//            if (maxCellBPD > row.getExplicitHeight().max) {
>> +//                log.warn(FONode.decorateWithContextInfo(
>> +//                        "The contents of row " + (row.getIndex() + 1)
>> +//                        + " are taller than they should be (there is a"
>> +//                        + " block-progression-dimension or height constraint
>> +//                        + " on the indicated row)."
>> +//                        + " Due to its contents the row grows"
>> +//                        + " to " + maxCellBPD + " millipoints, but the row shouldn't get"
>> +//                        + " any taller than " + row.getExplicitHeight() + " millipoints.",
>> +//                        row.getTableRow()));
>> +//            }
>> +            if (log.isDebugEnabled()) {
>> +                log.debug("  height=" + rowHeights[rgi] + " explicit=" + explicitRowHeight);
>>              }
>>          }
> <snip/>
>
>
> Jeremias Maerki
>

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