353699 - mBoundNode used in SetMozTypeAttribute should use GetBoundNode instead

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

I noticed after checking in the patch for bug 331984 that I'm getting NS_ENSURE_SUCCESS failures for xf:output's that use @value instead of @ref.  My code (nsXFormsControlStubBase::GetBoundBuiltinType) uses GetTypeForControl() which gets the type of the bound node.  This is failing since GetBoundNode won't find a bound node.  Since xf:output is using @value, that isn't too surprising.  However, the call to GetBoundBuiltinType from inside SetMozTypeAttribute is inside a test for mBoundNode.  So it turns out that mBoundNode can exist for this kind of output (needs to have it in order to provide context, is what the comment says).

So I'm thinking that to get rid of these errors, the right fix is to change the test for mBoundNode inside SetMozTypeAttribute so that it uses GetBoundNode instead.  This would have the side effect that xf:output with only @value will no longer have a @mozType:type set.  I think that this is the right solution and will give us the right behavior under the spec -> if someone has a custom control for xf:output when it is bound to a number, for example, this custom control should not be used if they are using @value instead of @ref, I don't think.

But I wanted to give everyone a heads up that I'm going to make this change unless someone depends on the current behavior and let's me know that it will break them.

Comment 1

[aaronr]

I think we'll have to make the change anyhow.  mBoundNode on an output with only @value will be its context node (the root node/document element of the default instance document for the model if the output isn't contained in another xforms element).  So we are, for this type of output, setting its type to be the type of its context node with is clearly wrong.  So I need to change SetMozTypeAttribute.

The only possible issue I see with this is then this output would have no type associated with it.  But that would be the same as any other unbound control in our processor.

Comment 2

[aaronr]

Attachment: patch 1

fixed SetMozTypeAttribute to use GetBoundNode instead of mBoundNode

Comment 3

[alexander :surkov (:asurkov)]

The patch looks like workaround, why mBoundNode can be not null when xf:output use @value attribute?

Comment 4

[aaronr]

To be more accurate, nsXFormsOutputElement having a non-null mBoundNode when there are no binding attributes is the workaround :-)  Mine is more of an equivalent alternative that happens to get a more accurate result :-)

Look at Allan's patch for bug 313118.  You'll see in his comments to nsXFormsOutputElement that the bound node is used to help determine context when nodes use relative xpath expressions for binding and other xpath expressions (like @value on xf:output).  I assume that he chose this method to solve his problem to refrain from having to introduce another member value for every control that is going to be equivalent to mBoundNode in 90% of the cases.  hasBoundNode and getBoundNode both work correctly so the only thing we need to keep in mind is that mBoundNode might not ALWAYS be the bound node so using it as a short cut is risky.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 239585 [details] [diff] [review]
patch 1

I think this is ok.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 239585 [details] [diff] [review]
patch 1

Then I guess that's fine.

Comment 7

[aaronr]

checked into trunk

Comment 8

[aaronr]

checked into 1.8.0 on 2006/09/27

Comment 9

[aaronr]

checked into 1.8 branch on 2006/11/21