mBoundNode used in SetMozTypeAttribute should use GetBoundNode instead

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
12 years ago
2 years ago

People

(Reporter: aaronr, Assigned: aaronr)

Tracking

({fixed1.8.0.8, fixed1.8.1.1})

Trunk
x86
All
fixed1.8.0.8, fixed1.8.1.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
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.
(Assignee)

Comment 1

12 years ago
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.
(Assignee)

Comment 2

12 years ago
Created attachment 239585 [details] [diff] [review]
patch 1

fixed SetMozTypeAttribute to use GetBoundNode instead of mBoundNode
Attachment #239585 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

12 years ago
Blocks: 334603

Comment 3

12 years ago
The patch looks like workaround, why mBoundNode can be not null when xf:output use @value attribute?
(Assignee)

Comment 4

12 years ago
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

12 years ago
Comment on attachment 239585 [details] [diff] [review]
patch 1

I think this is ok.
Attachment #239585 - Flags: review?(Olli.Pettay) → review+

Comment 6

12 years ago
Comment on attachment 239585 [details] [diff] [review]
patch 1

Then I guess that's fine.
Attachment #239585 - Flags: review+
(Assignee)

Comment 7

12 years ago
checked into trunk
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Comment 8

12 years ago
checked into 1.8.0 on 2006/09/27
Keywords: fixed1.8.0.8
(Assignee)

Comment 9

11 years ago
checked into 1.8 branch on 2006/11/21
Keywords: fixed1.8.1.1
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.