Hook up Schema Validation to XForms

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: Doron Rosenberg (IBM), Assigned: Doron Rosenberg (IBM))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

14 years ago
I finally got XForms and Schema Validation to talk to eachother and got XForms
to set the eFlag_CONSTRAINT based on the validation result.

To set the eFlag_CONSTRAINT, I had to add:
nsXFormsMDGEngine::MarkNodeAsInvalid(nsIDOMNode* aContextNode, PRBool aValue)

The code I am about to attach is ugly, but I am wondering if its the right way
to go.
(Assignee)

Comment 1

14 years ago
Created attachment 171933 [details] [diff] [review]
current code, does not contain all changes required to make it work.

Comment 2

14 years ago
Even though this patch isn't EVERYTHING to make it work, it shows the process
that we are thinking of using right now to get/update state between the MDG and
model.  What we really need to know is if this is the best way to update the
nsXFormsNodeState (i.e ask the MDG to update the flags for us) or should the
model get the nodestate and update the flag itself.
Status: NEW → ASSIGNED

Comment 3

14 years ago
(In reply to comment #0)
> I finally got XForms and Schema Validation to talk to eachother and got XForms
> to set the eFlag_CONSTRAINT based on the validation result.
> 
> To set the eFlag_CONSTRAINT, I had to add:
> nsXFormsMDGEngine::MarkNodeAsInvalid(nsIDOMNode* aContextNode, PRBool aValue)
> 
> The code I am about to attach is ugly, but I am wondering if its the right way
> to go.

imho, not exactly :) It should be part of the inner workings of the MDG
revalidate-cycle, but I haven't given it much thought.
(Assignee)

Comment 4

14 years ago
The problem I ran into is that all methods to get the nodestate that can be
called from the model element return consts, so I couldn't call ->Set() on it.
(Assignee)

Comment 5

14 years ago
I added this method now:

nsresult
nsXFormsMDGEngine::SetNodeStateFlag(nsIDOMNode* aNode, PRUint16 aFlag, PRBool
aValue)
{
  nsXFormsNodeState* ns = GetNCNodeState(aNode);
  NS_ENSURE_TRUE(ns, NS_ERROR_FAILURE);

  ns->Set(aFlag, aValue);
  return NS_OK;
}

so it can be shared to set flags.

Comment 6

14 years ago
(In reply to comment #5)
> I added this method now:
> 
> [SNIP]

Sorry, I haven't got around to this yet, but the whole idea of the const return
of the NodeState is that only the MDG determines the states, so having such a
function is against (my idea of) the design. The validation needs to be done
inside the MDG as part of the Recalculate() function, as it only needs to be
done when a node value changes.
(Assignee)

Comment 7

14 years ago
(In reply to comment #6)
> (In reply to comment #5)
> > I added this method now:
> > 
> > [SNIP]
> 
> Sorry, I haven't got around to this yet, but the whole idea of the const return
> of the NodeState is that only the MDG determines the states, so having such a
> function is against (my idea of) the design. The validation needs to be done
> inside the MDG as part of the Recalculate() function, as it only needs to be
> done when a node value changes.

Ee put a ValidateNode method in nsXFormsModelElement, so I was confused why we
needed it if the validation should go into MDG.  Should MDG link against
nsXFormsSchemaValidation and call it or should MDG validate through the model?
(Assignee)

Comment 8

14 years ago
Created attachment 172389 [details] [diff] [review]
Another try

I was blanking out, I think I got it this time.

nsXFormsMDGEngine::Recalculate() calls nsXFormsModelElement::ValidateNode() now
and it works fine.

Note that this requires my schema validation changes that are still in the
review process, so you can't actually test it.
(Assignee)

Updated

14 years ago
Attachment #171933 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #172389 - Flags: review?(allan)

Comment 9

14 years ago
If there is a dependency on schema changes, could you please note that in the
dependent bug list?  I'm having trouble keeping the schema bugs seperated in my
head.  Thanks.
(Assignee)

Comment 10

14 years ago
note that this builds fine without the code in bug 223097 if a schema is loaded
in the model.  If no schema is loaded by the model, it will throw an
NS_ERROR_SCHEMAVALIDATOR_NO_SCHEMA_LOADED.
Depends on: 223097

Comment 11

14 years ago
Comment on attachment 172389 [details] [diff] [review]
Another try

>Index: extensions/xforms/nsXFormsMDGEngine.cpp
...
>@@ -331,7 +334,7 @@
> #ifdef DEBUG_XF_MDG
>     nsAutoString domNodeName;
>     g->mContextNode->GetNodeName(domNodeName);
>-    
>+

Nit: whitespace changes.

>     printf("\tNode #%d: This=%p, Dirty=%d, DynFunc=%d, Type=%d, Count=%d, Suc=%d, CSize=%d, CPos=%d, Next=%p, domnode=%s\n",
>            i, (void*) g, g->IsDirty(), g->mDynFunc, g->mType,
>            g->mCount, g->mSuc.Count(), g->mContextSize, g->mContextPosition,
>@@ -380,17 +383,26 @@
>         rv = BooleanExpression(g, constraint);
>         NS_ENSURE_SUCCESS(rv, rv);
>       }
>-      ///
>-      /// @todo Schema validity should be checked here (XXX)
>-              
>+
>       if (ns->IsConstraint() != constraint) {
>         ns->Set(eFlag_CONSTRAINT, constraint);
>         ns->Set(eFlag_DISPATCH_VALID_CHANGED, PR_TRUE);
>         NS_ENSURE_TRUE(aChangedNodes->AddNode(g->mContextNode),
>                        NS_ERROR_FAILURE);
>       }
>+
>+      // Validate the node
>+      if (mModel) {
>+        PRBool isValid;
>+        mModel->ValidateNode(g->mContextNode, &isValid);
>+
>+        nsXFormsNodeState* ns = GetNCNodeState(g->mContextNode);
>+        NS_ENSURE_TRUE(ns, NS_ERROR_FAILURE);
>+
>+        ns->Set(eFlag_CONSTRAINT, isValid);
>+      }

Replace with:
if (mModel && constraint) {
  mModel->ValidateNode(g->mContextNode, &constraint);
}
and move it up where the original comment was.

>       break;
>-      
>+

Nit: More whitespace changes.

>Index: extensions/xforms/nsXFormsMDGEngine.h
...
>+
>+  /** The model that created the MDG*/
>+  nsIModelElementPrivate *mModel;

Nit: Add a space before '*/'

>   /**
>    * Set of nodes that are marked as changed, and should be included in
>@@ -323,7 +327,7 @@
>   /**
>    * Initializes the internal structures. Needs to be called before class is used!
>    */
>-  nsresult Init();
>+  nsresult Init(nsIModelElementPrivate *aModel);

Nit: Add a @param aModel <insert description here>, and make the ever-whining
Doxygen-user happy :)

>Index: extensions/xforms/nsXFormsModelElement.cpp
...
>+nsXFormsModelElement::ValidateNode(nsIDOMNode *aInstanceNode, PRBool *aResult)
> {
...
>+
>+  printf(" Is valid: %d \n", isValid ? 1 : 0);
>+

Remove before check in.

>+  *aResult = isValid;

Null-check aResult.

>Index: extensions/xforms/nsIModelElementPrivate.idl
...
>-  PRBool validateNode(in nsIDOMNode instanceNode);
>+  PRBool validateNode(in nsIDOMNode aInstanceNode);

I'm not sure what is the right thing here for IDLs?

I haven't tested the code at all, but I need to change something in the MDG I
think, the type will only be checked when there is a constraint on the node.

I'm sorry I haven't gotten around and hacked it together, but I thought I could
"just do" index()/<setindex> before looking at this. It proved more difficult
than I thought so it took (and takes) my time.

But with the above changes, r=me.
Attachment #172389 - Flags: review?(allan) → review+
(Assignee)

Comment 12

14 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.