Bug 47541 - Wrong handling of "retained" conditionality when space-before is inherited
Summary: Wrong handling of "retained" conditionality when space-before is inherited
Status: REOPENED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: fo tree (show other bugs)
Version: all
Hardware: All All
: P3 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-16 06:07 UTC by Vincent Hennebert
Modified: 2012-04-07 01:52 UTC (History)
0 users



Attachments
Sample FO file showing the problem (2.29 KB, text/plain)
2009-07-16 06:07 UTC, Vincent Hennebert
Details
The true sample file that shows the problem (2.22 KB, text/plain)
2009-07-16 06:08 UTC, Vincent Hennebert
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Hennebert 2009-07-16 06:07:06 UTC
Created attachment 23993 [details]
Sample FO file showing the problem

See attached file: block b4 inherits the space-before property from the parent block and overrides the .conditionality component to "retain". This results into a space before the first block, at the top of the first page. Normally block 4 should start the second page with a space before it. The bug doesn't occur if all the components of the space are specified on the block instead of being inherited.
Comment 1 Vincent Hennebert 2009-07-16 06:08:55 UTC
Created attachment 23994 [details]
The true sample file that shows the problem
Comment 2 Andreas L. Delmelle 2009-07-16 12:36:44 UTC
Not a 100% sure, but I think this behavior has been discussed (way back when), and this is actually not a bug.
By specifying the .conditionality component, you implicitly set the other property components to their initial value. Either all the components are inherited, or none.

To achieve the desired result, it is wrong to rely on implicit inheritance. Instead, specify a value of 'inherit' for the .length component to avoid using the initial value (0pt).
Comment 3 Andreas L. Delmelle 2009-07-16 14:03:20 UTC
Just some additional notes explaining the behavior, that even though it seems unexpected, is actually OK.

For compound properties, specifying only one of the components is equivalent to setting all the other components to their initial value. They are not initialized with the inherited value, since the components are NOT(!) separate properties. Implicit inheritance is always defined on the level of the complete property. If one of the components appears as a specified value, the entire property is no longer inherited. If we want to inherit some of the components and override others, we need to use explicit values of 'inherit'. (Seems to be precisely one of the prime use-cases for that keyword)

IOW:
<block space-before="2em">
  <block space-before.conditionality="retain">

is semantically equivalent to

<block space-before.optimum="2em"
              space-before.conditionality="discard"
              space-before.precedence="0">
  <block space-before.optimum="0pt"
                space-before.conditionality="retain"
                space-before.precedence="0">

To achieve the expected result, the correct spec would be:
<block space-before="2em">
  <block space-before.optimum="inherit" space-before.conditionality="retain">

Technically, in FOP (see also the Wiki about property resolution) specified values are always processed first and added to the PropertyList. Later, when binding the PropertyList to the FObj, if there was no specified value for a given applicable property, inheritance will always be tried before reverting to the initial value.
When processing the specified properties, and encountering a specified component, we first check if the compound property already exists on the PropertyList. In this context, we don't try inheritance, as we're actually looking for a base property that possibly resulted from other components that were processed before (= appeared earlier in the Attributes). 
If it does not exist, then we generate that base property, also bypassing inheritance, since we're creating an instance for a specified value.

I think this is completely in line with the Recommendation (5.1.4), which hints at the inherited value actually being a fallback for an absent specified value (not a replacement for the initial value; note the order of the resolution rules in 5.1.1).
Comment 4 Vincent Hennebert 2009-07-17 07:04:35 UTC
That doesn't justify the space before the first block on the first page. It clearly looks like some object is directly passed to the child FO, instead of a copy of it. Setting the conditionality to "retain" on block 4 seems to have set it also to the surrounding block.

Also:

(In reply to comment #3)
> Just some additional notes explaining the behavior, that even though it seems
> unexpected, is actually OK.
> 
> For compound properties, specifying only one of the components is equivalent to
> setting all the other components to their initial value. They are not
> initialized with the inherited value, since the components are NOT(!) separate
> properties. Implicit inheritance is always defined on the level of the complete
> property.

This is not what the following sentence from section 5.11 says: "Compound values of properties are inherited as a unit and not as individual components. After inheritance any complete form specification for a component is used to set its value." So non-specified components take the inherited value.

Anyway, that doesn't apply to space-before since it's not an inheritable property.


> If one of the components appears as a specified value, the entire
> property is no longer inherited. If we want to inherit some of the components
> and override others, we need to use explicit values of 'inherit'. (Seems to be
> precisely one of the prime use-cases for that keyword)

Which is what is done in the sample FO file. According to the following sentence in section 5.11: "Short forms may be used together with complete forms; the complete forms have precedence over the expansion of a short form." the result for block b4 should be the following one:
  space-before.minimum=10pt
  space-before.optimum=12pt
  space-before.maximum=50pt
  space-before.precedence=0
  space-before.conditionality=retain


<snip/> 
> I think this is completely in line with the Recommendation (5.1.4), which hints
> at the inherited value actually being a fallback for an absent specified value
> (not a replacement for the initial value; note the order of the resolution
> rules in 5.1.1).

Like above, since space-before is not an inheritable property, section 5.1.4 doesn't apply here.


Vincent
Comment 5 Andreas L. Delmelle 2009-07-17 08:04:24 UTC
(In reply to comment #4)

Sorry, mixing things up with the inherited indent properties. Should have looked at the sample, of course...

> That doesn't justify the space before the first block on the first page. It
> clearly looks like some object is directly passed to the child FO, instead of a
> copy of it. Setting the conditionality to "retain" on block 4 seems to have set
> it also to the surrounding block.

OK, now I see it. The SpaceProperty instance is re-used/shared between the blocks, due to the value of inherit, but when the additional component is specified on the fourth block, it also sets the component for all the other blocks. Specifying a value other than inherit, or specifying it for the individual components, generates a separate instance, so there are no side-effects. SpaceProperty is one type that is not cached yet, so even identical property specs will yield separate instances (unless inheritance of the base property is used, obviously).

Not sure how easy it would be to fix this behavior, yet. Could be as simple as modifying the behavior in PropertyList.findBaseProperty() to conditionally duplicate the instance...
Comment 6 Glenn Adams 2012-04-07 01:44:10 UTC
resetting P2 open bugs to P3 pending further review