278471 - reset event on Model element doesn't work

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

The model element doesn't currently handle the xforms-reset event.

I will attach a testcase that shows the problem.  

We need to be sure to take into account when we are using external instance
data, too.  Not just as simple as re-populating the instance documents with the
instance elements in the DOM :)  Can't just read in again from the external
source file since it may have changed from underneath us.  Actually that is a
good compatibility testcase to make sure we behave like the other
implementations...do they handle reset correctly or do they "reinitialize"?  And
MDG will probably have to be notified in some way since the data will change and
even potentially the data type of the data.

Comment 1

[aaronr]

Attachment: testcase that shows the problem

Testcase that recreates the problem.

Comment 2

[Darin Fisher]

from section 4.3.8 of the xforms spec:

  The default action for this event results in the following:

  The instance data is reset to the tree structure and values it had 
  immediately after having processed the xforms-ready event. Then, the events 
  xforms-rebuild, xforms-recalculate, xforms-revalidate and xforms-refresh are 
  dispatched to the model element in sequence.

I agree that we probably don't want to rely on being able to reload the same
instance data.  In fact, it is entirely possible that the instance data may have
been modified by a submission.  It's also possible that inline instance data may
have been modified via the DOM, however unlikely, so reloading inline instance
data from the DOM is also probably not right.  I think this means that we need
to keep a copy of the original instance data in memory, which is just plain
unfortunate.  It's too bad the XForms spec didn't say that the instance data
will be reset from source or something like that :-/

Comment 3

[aaronr]

Attachment: more exact testcase

other testcase shows another XForms bug dealing with delaying actions inside an
xforms:action element.	This new testcase only does set value and reset.

Comment 4

[aaronr]

Attachment: proposed fix

Nobody could come up with a better, fool-proof idea than just keeping an
original copy of every instance document in memory, so that is what I did.  On
xforms-ready I backup the model's instance data, instance element by instance
element.  On xforms-reset, I restore the model's original instance data,
instance element by instance element.

What sux here is that instance data could be potentially very large, especially
when loaded externally.  We eventually need a better idea.  But whatever it is,
needs to restore instance data properly, even when data is monkied with by
script.

Comment 5

[Allan Beaufour]

(In reply to comment #4)
> Created an attachment (id=172419) [edit]
> proposed fix

<stream-of-consciousness>
nsXFormsModel::Rebuild() is still a NOOP. We need to reattach all elements,
which is something like running ProcessBind() and doing a
nsIXFormsControl::Bind() on all controls, or something in that direction, or the
likes.
</stream-of-consciousness>

Comment 6

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

Comment on attachment 172419 [details] [diff] [review]
proposed fix


>+  if(mDocument && mOriginalDocument) {
>+    nsCOMPtr<nsIDOMNode> newNode, instanceRootNode;
>+    nsCOMPtr<nsIDOMElement> instanceRoot;
>+    rv = mDocument->GetDocumentElement(getter_AddRefs(instanceRoot));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    NS_ENSURE_TRUE(instanceRoot, NS_ERROR_FAILURE);
>+    instanceRootNode = do_QueryInterface(instanceRoot);

instanceRoot is already an nsIDOMNode.

>+
>+    nsCOMPtr<nsIDOMNode> nodeReturn;
>+    rv = instanceRootNode->CloneNode(PR_TRUE, getter_AddRefs(newNode)); 
>+    if(NS_SUCCEEDED(rv)) {
There should a mOriginalDocument->Import() here, but Mozilla is a bit buggy...
>+      rv = mOriginalDocument->AppendChild(newNode, getter_AddRefs(nodeReturn));
>+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), 



And then Allan's comments. Make sure the nsXFormsModelElement::Rebuild() does
the right thing.

With this patch I get the following warning:
WARNING: XForms: There are loops in the MDG
, file nsXFormsMDGEngine.cpp, line 516

Comment 7

[aaronr]

Ok, I'll fix the patch about instanceRoot.  I'd say that rebuild not working is
a seperate bug, which I've opened ->
https://bugzilla.mozilla.org/show_bug.cgi?id=279957.  As far as xforms-reset
handling, this bug is calling the right things in the right order.

beaufour says that getting Rebuild() to work right will get rid of the warning
that you mentioned.

Comment 8

[aaronr]

Attachment: fix with smaug's decision

removed instanceRootNode.

Comment 9

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

Comment on attachment 172531 [details] [diff] [review]
fix with smaug's decision

This looks ok, but Bug 279957 should probably be fixed before check in this
one.

Comment 10

[aaronr]

Attachment: update to trunk

same patch, just updated to the trunk.

Comment 11

[Brian Ryner (not reading)]

checked in