280017 - Let remaining controls implement nsIContextControl

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

Right now, only repeat, group and switch set their context through. All elements
that have a node binding should set context, coincidently :) that should be the
same elements that inherit from nsXFormsControl so it should be possible to put
in nsXFormsControlStub.

Comment 1

[Allan Beaufour]

Attachment: Patch v1

This patch makes nsIXFormsControl inherit nsIXFormsContextControl, and
implements it in nsXFormsControlStub, thus setting up the context for all
nsIXFormsControl-controls.

I believe it is rather crucial to have in the beta, as wrong contexts could
break a lot of forms.

Comment 2

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

Comment on attachment 172814 [details] [diff] [review]
Patch v1

>+NS_IMPL_ISUPPORTS_INHERITED2(nsXFormsControlStub,
>+                             nsXFormsXMLVisualStub,
>+                             nsIXFormsContextControl,
>+                             nsIXFormsControl);

Remove ';'

>+  nsXFormsSwitchElement() : mAddingChildren(PR_FALSE) {};
And here


>+NS_IMPL_ISUPPORTS_INHERITED1(nsXFormsSwitchElement,
>+                             nsXFormsControlStub,
>+                             nsIXFormsSwitchElement);
And here

> + * @bug If a control has a model attribute, but no binding attributes we fail
> + *      to set this as the context for children. We need to return the contextnode
> + *      from EvaluateNodeBinding in that case, and return that in GetContext(). (XXX)
Would it be difficult to fix this now?

The changes to Switch element (mAddingChildren) look right though they are not
related to this bug.

Comment 3

[Allan Beaufour]

(In reply to comment #2)
> > + * @bug If a control has a model attribute, but no binding attributes we fail
> > + *      to set this as the context for children. We need to return the
contextnode
> > + *      from EvaluateNodeBinding in that case, and return that in
GetContext(). (XXX)
> Would it be difficult to fix this now?

You never know before you attack it, and I've learned my lesson with "the patch
from hell" :) So, I'd rather do a new bug on that. I'll create a new one, and
include a ref to it in the code.

Comment 4

[Allan Beaufour]

Attachment: Patch with smaug's comments

* removed the ';''s
* added a ref. to the bug with the "lone model" attribute bug 280366
* updated to current CVS

Comment 5

[Allan Beaufour]

Attachment: Updated to current CVS

Comment 6

[Brian Ryner (not reading)]

Comment on attachment 173067 [details] [diff] [review]
Updated to current CVS

>--- xforms/nsXFormsControlStub.cpp	2005-02-01 09:47:24.204850760 +0100
>+++ xforms.contextcontrol/nsXFormsControlStub.cpp	2005-02-01 11:08:56.882741648 +0100
>+NS_IMETHODIMP
>+nsXFormsControlStub::GetContext(nsAString      &aModelID,
>+                                nsIDOMNode    **aContextNode,
>+                                PRInt32        *aContextPosition,
>+                                PRInt32        *aContextSize)
>+{
>+  NS_ENSURE_ARG(aContextSize);
>+  NS_ENSURE_ARG(aContextPosition);
>+
>+  *aContextPosition = 1;
>+  *aContextSize = 1;
>+
>+  if (mBoundNode && aContextNode)
>+    CallQueryInterface(mBoundNode, aContextNode); // addrefs

I would return the error code if this QI fails.  Or, if it should never fail,
add an NS_ASSERTION(*aContextNode).

Looks fine otherwise.

Comment 7

[Allan Beaufour]

Fixed Brian's comment and checked in.