294441 - fix model-construct-done processing

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

We need to shore up our model-construct-done processing according to sec. 4.2.2
in the spec.  This includes: 

A) If the instance referenced on the form control existed when the first form
control was processed:

1. The binding expression is evaluated to ensure that it points to a node that
exists. If this is not the case then the form control should behave in the same
manner as if it had bound to a model item with the relevant model item property
resolved to false.

B) If the instance referenced on the form control did not exist when the first
form control for the same instance was processed:

1. For the first reference to an instance a default instance is created by
following the rules described below.
  a. A root instanceData element is created.
  b. An instance data element node will be created using the binding expression
from the user interface control as the name. If the name is not a valid QName,
processing halts with an exception (4.5.1 The xforms-binding-exception Event).
2. For the second and subsequent references to an instance which was
automatically created the following processing is performed:
  a. If a matching instance data node is found, the user interface control will
be connected to that element.
  b. If a matching instance data node is not found, an instance data node will
be created using the binding expression from the user interface control as the
name. If the name is not a valid QName, processing halts with an exception
(4.5.1 The xforms-binding-exception Event).

Comment 1

[aaronr]

Attachment: testcase that shows the problem

enter "abcd" in the first input, select an item from the xf:select item.  Click
on the submit button.  You should see both values echoed back inside a root
element named "instanceData".  You should also not see the text that says, "You
should not see this text".

Comment 2

[aaronr]

Attachment: first attempt

allows for lazy authoring and 'disabling' unbindable elements per 4.2.2

Comment 3

[aaronr]

Attachment: fixed testcase

fixed testcase.  First version was missing styling to hide 'disabled' controls

Comment 4

[aaronr]

Attachment: fixed testcase 2

previous versions of testcase suffered from bug 297692.  This version won't and
will still test lazy authoring and disabling non-bindable controls.

Comment 5

[aaronr]

This patch first makes a change on how we find instance elements for a given
model.  We used to just look for nsIInstanceElementPrivate node's as children of
the model node in the xforms document.  That won't work for lazy-authored
instance since these instance only live in memory.  So I created a list of
instance elements off of the model which will include both normal instances as
well as lazy authored instances.  Naturally, I had to change the areas of code
that used the old document navigation method to use this new method.

The next thing I did was create a sister class to nsXFormsInstanceElement called
nsXFormsLazyInstanceElement which also implements nsIInstanceElementPrivate.  It
is pretty similar to its predecessor, even allowing for Backup and Restore so
that Reset will work with lazy authored instance, too.

Comment 6

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

Comment on attachment 186183 [details] [diff] [review]
first attempt


>+  NS_ENSURE_TRUE(model, NS_ERROR_FAILURE);
>+  nsCOMPtr<nsIInstanceElementPrivate> instance = 
>+                              NS_STATIC_CAST(nsIInstanceElementPrivate*, this);

Strange indentation. 2 spaces is enough.



>+      nsXFormsLazyInstanceElement *lazyInstance = 
>+                                             new nsXFormsLazyInstanceElement();
>+      lazyInstance->CreateLazyInstanceDocument(domDoc);

*Check OOM before using lazyInstance.
*indentation



>+                  rv = domdoc->CreateElementNS(namespaceURI,
>+                                               Substring(colon+1,end),
>+                                               getter_AddRefs(instanceDataEle));

CreateElementNS(namespaceURI, expr, getter_AddRefs(instanceDataEle)) should
work.

>+                nsCOMArray<nsIInstanceElementPrivate> *instList = nsnull;
>+                (*aModel)->GetInstanceList(&instList);
>+                nsCOMPtr<nsIInstanceElementPrivate> instance = 
>+                                                         instList->ObjectAt(0);

indentation

>+
>+class nsXFormsLazyInstanceElement : public nsXFormsStubElement,
>+                                    public nsIInstanceElementPrivate,
>+                                    public nsIInterfaceRequestor
>+{

Do you need nsIInterfaceRequestor for something ?

Comment 7

[aaronr]

> 
> >+                  rv = domdoc->CreateElementNS(namespaceURI,
> >+                                               Substring(colon+1,end),
> >+                                               getter_AddRefs(instanceDataEle));
> 
> CreateElementNS(namespaceURI, expr, getter_AddRefs(instanceDataEle)) should
> work.

But that would create an element like <foo:bar xmlns:foo="http://foo..."/>,
right?  I wanted to keep it as close to Novell's implementation as possible
since that seems the most right.  So this way I create an element like <bar
xmlns="http://foo..." />

> >+
> >+class nsXFormsLazyInstanceElement : public nsXFormsStubElement,
> >+                                    public nsIInstanceElementPrivate,
> >+                                    public nsIInterfaceRequestor
> >+{
> 
> Do you need nsIInterfaceRequestor for something ?
> 

I only had it because nsXFormsInstanceElement had it.  I've removed it.

Comment 8

[aaronr]

Attachment: next attempt

fixed comments

Comment 9

[aaronr]

Comment on attachment 186441 [details] [diff] [review]
next attempt

nother patch coming

Comment 10

[aaronr]

Attachment: nother attempt

changed lazy authoring per smaug's comments to create elements like <foo:bar
xmlns:foo="http://foo..."> since that seems to more closely follow the spec.

Comment 11

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

Comment on attachment 186511 [details] [diff] [review]
nother attempt


>+                nsCOMPtr<nsIInstanceElementPrivate> instance = 
>+                                                         instList->ObjectAt(0);

nit, indentation

Comment 12

[aaronr]

Comment on attachment 186511 [details] [diff] [review]
nother attempt

I will fix that nit before checkin.  Can you do second review, Allan?

Comment 13

[aaronr]

Attachment: last version

final version for checkin.  Fixed smaug's nit and updated to trunk.  Doron,
could you please check in for me?  Thanks.

Comment 14

[Doron Rosenberg (IBM)]

checked in.

Comment 15

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

Aaron, do you remember if there was a reason to extend nsIInterfaceRequestor?
I asked you to remove it, and you did in attachment 186441 [details] [diff] [review], but it is again
in attachment 186511 [details] [diff] [review].

Comment 16

[aaronr]

(In reply to comment #15)
> Aaron, do you remember if there was a reason to extend nsIInterfaceRequestor?
> I asked you to remove it, and you did in attachment 186441 [details] [diff] [review] [edit], but it is again
> in attachment 186511 [details] [diff] [review] [edit].
> 

Honestly, I don't remember exactly why I put it back in.  That was a while ago.
 I know that nsXFormsInstaceElement needs the interface to get notifications
when loading external data sources (SetNotifcationCallbacks requires it as a
parameter), but that shouldn't ever be a need for lazy authoring.  All I can
guess is that there is a place that uses or returns an 'instance element' that
could be either lazy or non-lazy and it was screwing up that code somehow not
having that interface.  Sorry that I don't have an exact reason.  I looked
through my emails and notes, but don't see anything that I wrote down.