We should have an nsIXFormsDelegate::SetRelevant() method, so we can set whether a control is relevant or not from XBL. This would be helpful for the upload element (bug 275453), so the 'disabled' upload control would always call |SetRelevant(false)| in the constructor.

I'd suggest SetDisabled() instead of SetRelevant. Relevancy has more to do with instance data than the control.

(In reply to comment #1) > I'd suggest SetDisabled() instead of SetRelevant. Relevancy has more to do with > instance data than the control. Or even something that does not share name with a MIP. SetControlActive(), or something?

Hmmm, if we want this to function properly, we need to do this the other way around. The control needs to interrogate the widget, or else the control will start by setting states and sending events for "correctly bound controls", i.e. xforms-enabled, etc. in Bind(). Then in Refresh() the widget will then inform about the "disabled state", and new events will be sent. That should not happen IMHO.

(In reply to comment #0) > We should have an nsIXFormsDelegate::SetRelevant() method, so we can set > whether > a control is relevant or not from XBL. This would be helpful for the upload > element (bug 275453), so the 'disabled' upload control would always call > |SetRelevant(false)| in the constructor. > Using Allan's approach we'll need to know when is a good time to ask the control about whether it should be disabled, readonly, etc. Javier, at what point will the upload (in this case) be able to determine what its disabled state should be? Do we need to wait until after it is bound to ask it? After it is refreshed? If all it needs is to be bound, then we should be able to ask the nsIXFormsUIWidget inside nsXFormsModelElement::SetState(). Probably right after it gets the intrinsic state from the nodestate. Just take that state and pass it into the widget and let the widget twiddle the state as it needs to and return back the results. I might even be able to use this for my in range/out of range code. Then I wouldn't need the nsIXFormsRangeUIAccessor interface that I thought that I was going to need.

(In reply to comment #4) > (In reply to comment #0) > > We should have an nsIXFormsDelegate::SetRelevant() method, so we can set > > whether > > a control is relevant or not from XBL. This would be helpful for the upload > > element (bug 275453), so the 'disabled' upload control would always call > > |SetRelevant(false)| in the constructor. > > > > Using Allan's approach we'll need to know when is a good time to ask the > control about whether it should be disabled, readonly, etc. Javier, at what > point will the upload (in this case) be able to determine what its disabled > state should be? Do we need to wait until after it is bound to ask it? After > it is refreshed? > > If all it needs is to be bound, then we should be able to ask the > nsIXFormsUIWidget inside nsXFormsModelElement::SetState(). Probably right > after it gets the intrinsic state from the nodestate. Just take that state and > pass it into the widget and let the widget twiddle the state as it needs to and > return back the results. Sounds wrong to me that we need the model to find out about nsIXFormsUIWidget. As I wrote in bug 329376 comment 4, mAppearDisabled should handle the relevancy. Afaik, the only things that a control can influence is that it should set range status (which has nothing to do with the model), or appear as disabled. Again, afaik, the only times that a control should appear disabled is when it is 1) not bound to a node or 2) that it is bound to a node of unsupported type. The approach now is to bind different widget implementations depending on the type. Maybe the C++ side of things could check the type, and set the type to "MagicUniqueNameThatMeansUnsupportedType"? I actually like that approach better.

(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #0) My first thought was asking the control whether it is relevant or not. But you are right...it is up to the processor to determine if a control is relevant or not. There are rules to relevancy and a control shouldn't be able to override them. > Afaik, the only things that a control can influence is that it should set range > status (which has nothing to do with the model), or appear as disabled. Again, > afaik, the only times that a control should appear disabled is when it is 1) > not bound to a node or 2) that it is bound to a node of unsupported type. > (3) when the node is bound with a relevancy rule that resolves to false(). And (4) when the control is contained by another control that is considered irrelevant. So #1, #3 and #4 are things that the processor can take care of without asking the control. #2 will require feedback from the control. Since a custom control could be written to be more restrictive than the spec. > The approach now is to bind different widget implementations depending on the > type. Maybe the C++ side of things could check the type, and set the type to > "MagicUniqueNameThatMeansUnsupportedType"? I actually like that approach > better. > But I bind a control like: foo|range { -moz-binding: url('controls.xml#myRange'); } changing the mozType to some odd value won't affect my control at all. And having nsXFormsDelegateStub calling widget->IsThisTypeOk(mozType) to see whether its bound type is ok is a lot of overhead for the few use cases that may ever need this. Maybe the accessor should not be the widget telling the control whether it is relevant or not, but maybe what types are ok for it? But then we have the headache of testing the type and probably even passing it on to the schema-validation code to see if the current type is derived from one of the safe types. Or do we not want to make this available for custom controls? Upload, range and input are the base controls that have type-restriction needs that I know of.

(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #0) > > Afaik, the only things that a control can influence is that it should set range > > status (which has nothing to do with the model), or appear as disabled. Again, > > afaik, the only times that a control should appear disabled is when it is 1) > > not bound to a node or 2) that it is bound to a node of unsupported type. > > > > (3) when the node is bound with a relevancy rule that resolves to false(). And > (4) when the control is contained by another control that is considered > irrelevant. > > So #1, #3 and #4 are things that the processor can take care of without asking > the control. #2 will require feedback from the control. Since a custom > control could be written to be more restrictive than the spec. I was not clear enough, I meant that: "relevancy not directly controlled by model". 1) is not in theory, but it is in practice in our case :) We are on the same page. > > The approach now is to bind different widget implementations depending on the > > type. Maybe the C++ side of things could check the type, and set the type to > > "MagicUniqueNameThatMeansUnsupportedType"? I actually like that approach > > better. > > > > But I bind a control like: > > foo|range { > -moz-binding: url('controls.xml#myRange'); > } > > changing the mozType to some odd value won't affect my control at all. I you are in a different namespace, I would say that you are definately on your own. > And having nsXFormsDelegateStub calling widget->IsThisTypeOk(mozType) to see > whether its bound type is ok is a lot of overhead for the few use cases that > may ever need this. Maybe the accessor should not be the widget telling the > control whether it is relevant or not, but maybe what types are ok for it? But > then we have the headache of testing the type and probably even passing it on > to the schema-validation code to see if the current type is derived from one of > the safe types. > > Or do we not want to make this available for custom controls? Upload, range > and input are the base controls that have type-restriction needs that I know > of. Looking at my calendar makes me very pragmatic... It think we should have support for whatever makes it possible for us to implement standard 1.0 controls functionality. That said, maybe it should not be the type attribute that is used, but an additional attribute. That at least makes the type available to a custom control for doing whatever it pleases, even for unsupported types -- and even though it cannot manipulate the MIP.

ok, so let me see if I understand where we've left the train of thought in this bug. In summary, it shouldn't be possible for a control to determine for itself if it is relevant or irrelevant. A control should only be able to determine if it can handle the type of the node that it is bound to. If it cannot, then it should not be rendered. If it can, then at least the 'supported type' logic shouldn't make it irrelevant. Have I got that right so far? Assuming that I am on the right track up to this point, what do you want this new attribute to do Allan? Or how should the xforms processor treat it if it finds it? Or doesn't find it? I think that is where I'm lost right now. Kinda related to this bug (or discussions that came up in this bug) I've started a patch that will allow uploads to bind to any type that is one of the three supported types (anyuri, base64binary and hex) AND any type derived from one of these three builtin types. So a skinned upload could bind to a more restrictive moztype foo:bar and still be able to display. Hopefully once that patch is done it might be of some use to bug 316691, too.

(In reply to comment #8) > ok, so let me see if I understand where we've left the train of thought in this > bug. In summary, it shouldn't be possible for a control to determine for > itself if it is relevant or irrelevant. A control should only be able to > determine if it can handle the type of the node that it is bound to. If it > cannot, then it should not be rendered. If it can, then at least the > 'supported type' logic shouldn't make it irrelevant. Have I got that right so > far? I'm actually not sure whether the specification specifies this. But: "Form controls must indicate when the bound instance data contains a value the form control is not capable of rendering. Control of this behavior should be made available to stylesheets." [http://www.w3.org/TR/2006/REC-xforms-20060314/index-all.html#ui-processing] "capable of rendering" could be interpreted as this case too. So I think that a control that is bound to a non-supported type should match ":disabled" too, and the stylesheets should determine what should be shown/not shown. > Assuming that I am on the right track up to this point, what do you want this > new attribute to do Allan? Or how should the xforms processor treat it if it > finds it? Or doesn't find it? I think that is where I'm lost right now. My idea is something along these lines (it's only a sketch): 1) The C++ control checks the supported types (per the spec.) and sets @mozType|supportedType="yes/no", and also mAppearDisabled. 2) xforms.css have xf|*[mozType|supportedType="no"] { -moz-binding: url('disabled-control'); } xf|range[mozType|supportedType="no"] { -moz-binding: url('disabled-range'); } Then we (and custom control authors) can then either do: xf|range[mozType|supportedType="yes"] { ... } for binding a generic "handle-it-all" control, or xf|range[mozType|type="http://www.w3.org/2001/XMLSchema#anyURI"] { ... } for type-specific controls. That way we leave the door open for people that want to render types that are actually not supported by the spec, since we do not "override" the @type. There still needs to be a scriptable SetAppearDisabled() though then. (I might have missed something... it's Monday.)

Hmmm, I don't know about the control matching :disabled. According to the spec, ":enabled & :disabled [CSS3] Selects any form control bound to a node with the model item property relevant evaluating to true or false (respectively)." Since the instance data node would still be submitted even if the type isn't derived from the proper built-in types, I'd say that it must be relevant. We can have the default style for xf|* [mozType|supportedType="no"] be display: none. And if we only set @supportedType from the c++ side during Bind(), a skinned control could change @supportedType to "yes" during refresh if it decided it liked the type and the control would show up.

(In reply to comment #10) > Hmmm, I don't know about the control matching :disabled. According to the > spec, ":enabled & :disabled [CSS3] Selects any form control bound to a node > with the model item property relevant evaluating to true or false > (respectively)." Since the instance data node would still be submitted even if > the type isn't derived from the proper built-in types, I'd say that it must be > relevant. Good point. I agree. > We can have the default style for xf|* [mozType|supportedType="no"] be display: > none. And if we only set @supportedType from the c++ side during Bind(), a > skinned control could change @supportedType to "yes" during refresh if it > decided it liked the type and the control would show up. Check.

fix here was to write a function called GetBuiltinType in nsXFormsModelElement that can decipher from a simpleType or complexType if it were derived from a builtin type and if so, which type. Then I changed nsXFormsUploadElement::Refresh to get the builtin type and if it were one of the three acceptable type, then set mozType:supportedtype="yes". If not, I remove the mozType:supportedType if it exists so that we don't show an upload. One question I have for reviewers is should GetBuiltinType maybe be supported as scriptable so that skinned controls can get to it?

(In reply to comment #15) > fix here was to write a function called GetBuiltinType in nsXFormsModelElement > that can decipher from a simpleType or complexType if it were derived from a > builtin type and if so, which type. Then I changed > nsXFormsUploadElement::Refresh to get the builtin type and if it were one of > the three acceptable type, then set mozType:supportedtype="yes". If not, I > remove the mozType:supportedType if it exists so that we don't show an upload. I'm not sure I like @supportedtype attribute. I think it's better to have to attributes @type and @baseType like Allan proposed in bug 316691. I guess such behaviour is more flexible. In this case if someone wants to create custom upload (or range or something else) widget, then he can use @baseType for it. > One question I have for reviewers is should GetBuiltinType maybe be supported > as scriptable so that skinned controls can get to it? > If we will have @baseType attribute then I guess we don't need GetBuiltinType() is sriptable. -*:disabled { +*:disabled, +html|*:root xf|*[mozType|supportedtype="no"] { display: none; } Why is it needed?

(In reply to comment #16) > (In reply to comment #15) > > fix here was to write a function called GetBuiltinType in nsXFormsModelElement > > that can decipher from a simpleType or complexType if it were derived from a > > builtin type and if so, which type. Then I changed > > nsXFormsUploadElement::Refresh to get the builtin type and if it were one of > > the three acceptable type, then set mozType:supportedtype="yes". If not, I > > remove the mozType:supportedType if it exists so that we don't show an upload. > > I'm not sure I like @supportedtype attribute. I think it's better to have to > attributes @type and @baseType like Allan proposed in bug 316691. I guess such > behaviour is more flexible. In this case if someone wants to create custom > upload (or range or something else) widget, then he can use @baseType for it. > But I think that you are assuming that it is just the base type that a custom control will be interested in. And I also think that you are assuming that the custom control widget author is the same person as the form author or that they have at least talk to each other. Consider this scenario. We have an instance node of type employer:type4. Assume that employer:type3 is extended from employer:type2 which is extended from builtin:type1. And employer:type4 is directly extended from builtin:type1. Using your suggestion, we'd set baseType to be builtin:type1, but my custom control might only be able to render data being derived from employer:type2. Now using script I can figure out if employer:type4 is extended from employer:type2, and I see that it isn't. Using the baseType approach, how does the custom widget tell the form that the type is not one that it can handle? > > One question I have for reviewers is should GetBuiltinType maybe be supported > > as scriptable so that skinned controls can get to it? > > > > If we will have @baseType attribute then I guess we don't need GetBuiltinType() > is sriptable. > > -*:disabled { > +*:disabled, > +html|*:root xf|*[mozType|supportedtype="no"] { > display: none; > } > > Why is it needed? > If you have supportedtype="yes", then we really need to be able to handle supportedtype="no". And making it display:none seems consistent to our current behavior of hiding the control if it isn't binding to a supported type.

(In reply to comment #17) > > Consider this scenario. We have an instance node of type employer:type4. > Assume that employer:type3 is extended from employer:type2 which is extended > from builtin:type1. And employer:type4 is directly extended from > builtin:type1. Using your suggestion, we'd set baseType to be builtin:type1, > but my custom control might only be able to render data being derived from > employer:type2. Now using script I can figure out if employer:type4 is > extended from employer:type2, and I see that it isn't. Using the baseType > approach, how does the custom widget tell the form that the type is not one > that it can handle? Very good example. But I guess we talk about little different things. 1) we should be able to recognize base type, in other case our controls will not work for type that is inherited from the one what control supports (the problem is from bug 316691). Your approach tends to use @supprotedtype attribute. But your implemetation sets this attribute in upload element code. It means we should write the same code for range element and e.t.c. The way proposed by Allan (it's using of @type/@baseType attributes) supposes we can have common code because model will sets @type and @baseType attributes in one place. 2) It's about your example. The first and probably uniquess way is using your interface. But it's very hackly to add custom control by this way using (because I should write some js script to reconnize needed base type and then I should add binding by some way). I can't see how it can be accomplished better.

(In reply to comment #15) > One question I have for reviewers is should GetBuiltinType maybe be supported > as scriptable so that skinned controls can get to it? > Yes, I think GetBuiltinType() should be scriptable (at least I can see it after Aaron's example from comment #17), but I'm not sure it should be defined in nsIXFormsNSModelElement interface or in xforms at all. Probably should it be accessible from some interface of schema module? I'm not sure I like GetBuiltinType() name. Is builtin type term from schema module? Probably GetBaseType() or something else?

(In reply to comment #19) > (In reply to comment #15) > > > One question I have for reviewers is should GetBuiltinType maybe be supported > > as scriptable so that skinned controls can get to it? > > > > Yes, I think GetBuiltinType() should be scriptable (at least I can see it after > Aaron's example from comment #17), but I'm not sure it should be defined in > nsIXFormsNSModelElement interface or in xforms at all. Probably should it be > accessible from some interface of schema module? > > I'm not sure I like GetBuiltinType() name. Is builtin type term from schema > module? Probably GetBaseType() or something else? > Actually, maybe we should change this to return ALL derived types. We could have it return an array of all types that it inherits from in a given order (from top to bottom or bottom to top), rather than just the builtin type that it inherits from. What do you think, would anyone use it that way or would they rather use just the builtin type?

(In reply to comment #20) > Actually, maybe we should change this to return ALL derived types. We could > have it return an array of all types that it inherits from in a given order > (from top to bottom or bottom to top), rather than just the builtin type that > it inherits from. What do you think, would anyone use it that way or would > they rather use just the builtin type? > Yes, if we will return base types then we should have a way to return all of them. Though I'm not sure about array. Probably we should have object "Type" that will have method to get base type. I don't fairly know xml schema but again at first sight it should be property of schema. I don't know is there peoples who will use it. But I look at xforms as at framework for application development (ugh! :)). We shouldn't cut down potential developers. And therefore this idea is very nice. Just probably we should leave this question and move it to separate bug for following discussion.

Comment on attachment 227506 [details] [diff] [review] patch 1 >Index: nsXFormsModelElement.cpp >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsModelElement.cpp,v >retrieving revision 1.127 >diff -u -8 -p -r1.127 nsXFormsModelElement.cpp >--- nsXFormsModelElement.cpp 6 Jun 2006 13:52:03 -0000 1.127 >+++ nsXFormsModelElement.cpp 29 Jun 2006 05:43:30 -0000 >@@ -2566,16 +2566,95 @@ nsXFormsModelElement::ForceRebind(nsIXFo > > rv = aControl->Refresh(); > // XXX: no rv-check, see bug 336608 > > // Refresh children > return RefreshSubTree(controlItem->FirstChild(), rebindChildren); > } > >+nsresult >+nsXFormsModelElement::GetBuiltinTypeInternal(nsISchemaType *aType, >+ PRUint16 *aBuiltinType) >+{ >+ NS_ENSURE_ARG(aType); >+ NS_ENSURE_ARG_POINTER(aBuiltinType); >+ *aBuiltinType = 0; >+ >+ PRUint16 sTypeValue = 0; >+ aType->GetSchemaType(&sTypeValue); >+ NS_ENSURE_STATE(sTypeValue); >+ >+ if (sTypeValue == nsISchemaType::SCHEMA_TYPE_SIMPLE) { >+ nsCOMPtr<nsISchemaSimpleType> simpleType(do_QueryInterface(aType)); >+ NS_ENSURE_STATE(simpleType); >+ PRUint16 simpleTypeValue; >+ simpleType->GetSimpleType(&simpleTypeValue); >+ NS_ENSURE_STATE(simpleTypeValue); >+ >+ if (simpleTypeValue == nsISchemaSimpleType::SIMPLE_TYPE_BUILTIN) { >+ nsCOMPtr<nsISchemaBuiltinType> builtinType(do_QueryInterface(aType)); >+ NS_ENSURE_STATE(builtinType); >+ builtinType->GetBuiltinType(aBuiltinType); >+ return NS_OK; >+ } >+ >+ if (simpleTypeValue == nsISchemaSimpleType::SIMPLE_TYPE_RESTRICTION) { >+ nsCOMPtr<nsISchemaRestrictionType> restrictionType(do_QueryInterface(aType)); >+ NS_ENSURE_STATE(restrictionType); >+ restrictionType->GetBaseType(getter_AddRefs(simpleType)); >+ return GetBuiltinTypeInternal(simpleType, aBuiltinType); >+ } >+ >+ if (simpleTypeValue == nsISchemaSimpleType::SIMPLE_TYPE_LIST) { >+ nsCOMPtr<nsISchemaListType> listType(do_QueryInterface(aType)); >+ NS_ENSURE_STATE(listType); >+ listType->GetListType(getter_AddRefs(simpleType)); >+ return GetBuiltinTypeInternal(simpleType, aBuiltinType); >+ } wouldn't a case statement be cleaner? Looks good to me. However, should there also be an mozType:schemaBaseType, allowing people to style say a decimal control different than an integer control? Would be tricky with schema lists where you can have several types though.

(In reply to comment #19) > (In reply to comment #15) > > > One question I have for reviewers is should GetBuiltinType maybe be supported > > as scriptable so that skinned controls can get to it? > > > > Yes, I think GetBuiltinType() should be scriptable (at least I can see it after > Aaron's example from comment #17), but I'm not sure it should be defined in > nsIXFormsNSModelElement interface or in xforms at all. Probably should it be > accessible from some interface of schema module? schema module's interfaces are frozen for branches since they are core. I'll leave GetBuiltinType for now until we can better define the different flavors that we may need. > > I'm not sure I like GetBuiltinType() name. Is builtin type term from schema > module? Probably GetBaseType() or something else? > I'm calling it GetBuiltinType as it will find the first nsISchemaSimpleType::SIMPLE_TYPE_BUILTIN that this type derives from. GetBaseType to me seems even more generic. For example, assume that we have a type called user:myType that derives from positiveInteger. If positiveInteger is the first nsISchemaSimpleType::SIMPLE_TYPE_BUILTIN that user:myType inherits from, then that is what GetBuiltinType would return. But I would think that a function called GetBaseType would return decimal for this same type since positiveInteger is derived from nonNegativeInteger which is derived from integer which is derived from decimal.

Alex, so assume I use the GetBaseType() approach and get the BASE type from user:myType and discover that it isn't one of the supported types for upload. Lets say it is xsd:decimal. What CSS rule should I use to hide the upload? If we set mozType:type or mozType:baseType to "invalid" and match against that to make it display:none, then don't we lose the information of what type the control really is?

Ok, I think I can summarize my current stumbling blocks this way: 1) if we go the baseType route, does baseType equal the first nsISchemaBuiltinType that user:myType is derived from, or is it the real base type? For example, if user:myType is derived from xsd:positiveInteger, is baseType xsd:positiveInteger or is it xsd:decimal? 2) if we are going to use xforms.css to hide uploads that aren't of one of the three supported types (anyURI, base64Binary, hexBinary), what should the CSS rule be?

(In reply to comment #25) > Ok, I think I can summarize my current stumbling blocks this way: > > 1) if we go the baseType route, does baseType equal the first > nsISchemaBuiltinType that user:myType is derived from, or is it the real base > type? For example, if user:myType is derived from xsd:positiveInteger, is > baseType xsd:positiveInteger or is it xsd:decimal? > > 2) if we are going to use xforms.css to hide uploads that aren't of one of the > three supported types (anyURI, base64Binary, hexBinary), what should the CSS > rule be? > Here we can use your idea about array of types. We can place @typesList (or something else) attribute that will hold list of inheritance chain types. In your example it will be "user:myType xsd:positiveInteger xsd:decimal". Then we can use css selection [att~=val] (http://www.w3.org/TR/CSS21/selector.html#q10) to choose proper binding. We can get some problem when we will have two bindings for different types that are in one inheritance chain. Though I guess we can use css selector combinations with :not pseudo-class for this. How about this? It will be broken if schema support multiple inheritance though. Does schema support multiple inheritance?

(In reply to comment #26) > (In reply to comment #25) > > Here we can use your idea about array of types. We can place @typesList (or > something else) attribute that will hold list of inheritance chain types. In > your example it will be "user:myType xsd:positiveInteger xsd:decimal". Then we > can use css selection [att~=val] (http://www.w3.org/TR/CSS21/selector.html#q10) > to choose proper binding. We can get some problem when we will have two > bindings for different types that are in one inheritance chain. Though I guess > we can use css selector combinations with :not pseudo-class for this. How about > this? > > It will be broken if schema support multiple inheritance though. Does schema > support multiple inheritance? > Not really multiple inheritance, but schema does support union types (http://www.w3.org/TR/xmlschema-2/#datatype-dichotomies) which is going to make this @baseType solution problematic. Using one of the schema examples in the dichotomies section, user:myType could have this inheritance chain one time: user:myType -> xsd:nonNegativeInteger -> xsd:integer -> xsd:decimal and another time it could have: user:myType -> xsd:string

(In reply to comment #27) > > Not really multiple inheritance, but schema does support union types > (http://www.w3.org/TR/xmlschema-2/#datatype-dichotomies) which is going to make > this @baseType solution problematic. Using one of the schema examples in the > dichotomies section, user:myType could have this inheritance chain one time: > > user:myType -> xsd:nonNegativeInteger -> xsd:integer -> xsd:decimal > > and another time it could have: > > user:myType -> xsd:string > Let we have xforms:somecontrol. If it is bounded to instance data of xsd:decimal type then we use the first it's implementation, when it is bound to instance data of xsd:string type then we sue the second implementation. Then xforms author creates union type combined xsd:decimal and xsd:string. What implementation should we use in that case? Should it be depended on current instance node value? How does GetBuiltinTypeInternal() method keep that case now?

(In reply to comment #28) > (In reply to comment #27) > > > > > Not really multiple inheritance, but schema does support union types > > (http://www.w3.org/TR/xmlschema-2/#datatype-dichotomies) which is going to make > > this @baseType solution problematic. Using one of the schema examples in the > > dichotomies section, user:myType could have this inheritance chain one time: > > > > user:myType -> xsd:nonNegativeInteger -> xsd:integer -> xsd:decimal > > > > and another time it could have: > > > > user:myType -> xsd:string > > > > Let we have xforms:somecontrol. If it is bounded to instance data of > xsd:decimal type then we use the first it's implementation, when it is bound to > instance data of xsd:string type then we sue the second implementation. Then > xforms author creates union type combined xsd:decimal and xsd:string. What > implementation should we use in that case? Should it be depended on current > instance node value? How does GetBuiltinTypeInternal() method keep that case > now? > I don't want to depend on the current instance node value because then we'll have to go through validation ALL the time (for every refresh of the control), which is just too inefficient. For right now I'm going to return an error if I detect a union in the chain. I've asked the WG for clarification on bound type restrictions in this thread: http://lists.w3.org/Archives/Public/www-forms/2006Jul/0057.html

(In reply to comment #29) > I don't want to depend on the current instance node value because then we'll > have to go through validation ALL the time (for every refresh of the control), > which is just too inefficient. For right now I'm going to return an error if I > detect a union in the chain. I've asked the WG for clarification on bound type > restrictions in this thread: > http://lists.w3.org/Archives/Public/www-forms/2006Jul/0057.html > I agree we shouldn't depend on current instance node value. I guess if someone wants to support union type for certain control then he should have custom control implementation for this. Then if we'll skip union support then can we get back to @typeList attribute idea?

(In reply to comment #30) > (In reply to comment #29) > > > I don't want to depend on the current instance node value because then we'll > > have to go through validation ALL the time (for every refresh of the control), > > which is just too inefficient. For right now I'm going to return an error if I > > detect a union in the chain. I've asked the WG for clarification on bound type > > restrictions in this thread: > > http://lists.w3.org/Archives/Public/www-forms/2006Jul/0057.html > > > > I agree we shouldn't depend on current instance node value. I guess if someone > wants to support union type for certain control then he should have custom > control implementation for this. Then if we'll skip union support then can we > get back to @typeList attribute idea? > I'm working towards that goal. I've almost got a patch ready.

As part of this patch, I'm going to make the default upload be display: none instead of xformswidget-upload-disabled. Does anyone think that this is wrong? I'm leaving in xformswidget-upload-disabled in case someone wants to use it as a custom control for disabled rather than hidden.

If I am using http://www.foo.com/datatypes#myDataType in the type list for a user defined type in a schema with the targetNamespace="http://www.foo.com/datatypes", then what do I use if there is no targetNamespace attribute on the schema? #myDataType? That is what I'm doing now. Waiting for feedback from some xforms WG guys I've asked.

This patch was done using Alex's desire for a typelist approach. Alex, please review this patch to make sure that it does what you need it to. Thanks.

same typelist patch, just fixed up after running through JST reviewer tool.

Comment on attachment 231272 [details] [diff] [review] patch cleaned up >Index: nsIModelElementPrivate.idl >+ * @param aType1 The original type >+ * @param aNamespace1 The original namespace URI Why is postfix '1' needed? >Index: nsXFormsDelegateStub.cpp >+ >+ // Note: even if we can't build the derived type list, we should leave on >+ // mozType attribute. We can still use the attr for validation, etc. But >+ // the type-restricted controls like range and upload won't display since >+ // they are bound to their anonymous content by @typeList Then should we remove mozTypeList attribute? >+ return; > } else { One of these statements is excess. >+ // this function recursively appends aType (and its base types) to >+ // aTypeArray. So it assumes aType isn't in the array already. Please use capital letter. That occurs in other places of the patch. >+ switch (aType) { >+ case nsISchemaBuiltinType::BUILTIN_TYPE_STRING: >+ typeString.AppendLiteral("string"); >+ break; >+ case nsISchemaBuiltinType::BUILTIN_TYPE_BOOLEAN: >+ typeString.AppendLiteral("boolean"); >+ break; >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DECIMAL: >+ typeString.AppendLiteral("decimal"); >+ break; >+ case nsISchemaBuiltinType::BUILTIN_TYPE_FLOAT: >+ typeString.AppendLiteral("float"); >+ break; >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DOUBLE: >+ typeString.AppendLiteral("double"); >+ break; >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DURATION: >+ typeString.AppendLiteral("duration"); >+ break; .. That looks like potential dublication. Can we use schema to find type name by it's constant using? If schema doesn't allow it then probably we should add it. Why I don't like it is schema gets type name and returns constant, then xforms do inverse action. How are xforms types handled here? >+ PRUint16 sTypeValue = 0; Prefix 's' usually is used for static variables I guess. >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUploadElement.cpp,v >+ // If it is not bound to 'anyURI', 'base64Binary', 'hexBinary', or an >+ // extension or derivation of one of these three types, then put an error in >+ // the console. CSS and XBL will make sure that the control won't appear in >+ // the form. > if (GetBoundType() == TYPE_DEFAULT) { >- xtfWrap->SetIntrinsicState(NS_EVENT_STATE_DISABLED); > nsXFormsUtils::ReportError(NS_LITERAL_STRING("uploadBoundTypeError"), > mElement); >- } else { >- xtfWrap->SetIntrinsicState(NS_EVENT_STATE_ENABLED); > } > >+ <!-- UPLOAD: HIDDEN --> >+ <binding id="xformswidget-upload-hidden"> >+ <content> >+ <html:span style="display: none;"> >+ <children/> >+ </html:span> >+ </content> >+ >+ </binding> >+ Can we just hide wrong upload control by css using? I don't know good schema, therefore I guess it's better to ask somebody else to check schema related code more precisely. Later I'll compile the patch and see how it works.

(In reply to comment #36) > (From update of attachment 231272 [details] [diff] [review] [edit]) > > >+ switch (aType) { > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_STRING: > >+ typeString.AppendLiteral("string"); > >+ break; > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_BOOLEAN: > >+ typeString.AppendLiteral("boolean"); > >+ break; > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DECIMAL: > >+ typeString.AppendLiteral("decimal"); > >+ break; > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_FLOAT: > >+ typeString.AppendLiteral("float"); > >+ break; > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DOUBLE: > >+ typeString.AppendLiteral("double"); > >+ break; > >+ case nsISchemaBuiltinType::BUILTIN_TYPE_DURATION: > >+ typeString.AppendLiteral("duration"); > >+ break; > > .. That looks like potential dublication. Can we use schema to find type name > by it's constant using? If we had an object we could. But we can't just from a PRUint16 builtin type value. > If schema doesn't allow it then probably we should add > it. Why I don't like it is schema gets type name and returns constant, then > xforms do inverse action. Right now there is no way in schema to pass in the builtinType PRUint16 value and get back a string. There is also no way to pass in a string and get back the PRUint16 builtinType. You have to have a nsISchemaBuiltinType object to get that information and you can't just construct one of those (the constructor is private). We can TRY to get these types of things put in schema, but since it is a core change and FF 2.0 is about to change, I don't see how we can do it through the interface. We might just have to make a static utility function if we can find a place to put it. > > How are xforms types handled here? They aren't. But I figured that they'd come in via bug 343122. But I could hardcode them in now. Putting XForms specific types in here would be another reason that this functionality can't live in schema land. > > >+ PRUint16 sTypeValue = 0; > > Prefix 's' usually is used for static variables I guess. > > >=================================================================== > >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUploadElement.cpp,v > > >+ // If it is not bound to 'anyURI', 'base64Binary', 'hexBinary', or an > >+ // extension or derivation of one of these three types, then put an error in > >+ // the console. CSS and XBL will make sure that the control won't appear in > >+ // the form. > > if (GetBoundType() == TYPE_DEFAULT) { > >- xtfWrap->SetIntrinsicState(NS_EVENT_STATE_DISABLED); > > nsXFormsUtils::ReportError(NS_LITERAL_STRING("uploadBoundTypeError"), > > mElement); > >- } else { > >- xtfWrap->SetIntrinsicState(NS_EVENT_STATE_ENABLED); > > } > > > > > >+ <!-- UPLOAD: HIDDEN --> > >+ <binding id="xformswidget-upload-hidden"> > >+ <content> > >+ <html:span style="display: none;"> > >+ <children/> > >+ </html:span> > >+ </content> > >+ > >+ </binding> > >+ > > Can we just hide wrong upload control by css using? I tried using display: none;, but that makes all uploads (including the ones we want to show) display none. But I'm open to suggestions if you can think of a way. I'm not exactly a CSS guru.

(In reply to comment #37) > (In reply to comment #36) > > (From update of attachment 231272 [details] [diff] [review] [edit] [edit]) > > If schema doesn't allow it then probably we should add > > it. Why I don't like it is schema gets type name and returns constant, then > > xforms do inverse action. > > Right now there is no way in schema to pass in the builtinType PRUint16 value > and get back a string. There is also no way to pass in a string and get back > the PRUint16 builtinType. My bad. There is a way to use the type string and namespace string to get back the builtin type value using nsISchemaCollection. But still no way to take the builtin type value and get back a string.

Talked to doron and mkaply about the builtin type to string converter today. We can't get any interface changes into FF 2.0 now since Beta1 has already gone out and webservices is a core component. I've thought about putting this in as a static function inside webservices (which we could still do today or tomorrow) and the more I think about it the more I don't like putting a dependency on webservices like that. So if everyone else concurs, then we are left with just keeping this an internal function or putting it in the trunk and using it from webservices in FF 3.0 and beyond. What does everyone think? Just leave the code as an internal function in XForms or have it live in webservices for FF 3.0 and use internal version just for FF 2.0?

(In reply to comment #39) > > What does everyone think? Just leave the code as an internal function in > XForms or have it live in webservices for FF 3.0 and use internal version just > for FF 2.0? > I guess we can use internal xforms version only since if will we have both verions then we should be forced to keep them synchronic. It's not nice. Later we can move xforms method into webservice (for sure if we will not forget it :)).

Aaron, can you update your patch to trunk?

updated to the trunk. Fixed most of Alex's comments. Leaving utility function in XForms.

Comment on attachment 231876 [details] [diff] [review] patch updated to trunk >Index: nsXFormsModelElement.cpp >+nsresult >+nsXFormsModelElement::AppendBuiltinTypes(PRUint16 aType, >+ nsStringArray *aTypeArray) >+{ >+ // this function recursively appends aType (and its base types) to >+ // aTypeArray. So it assumes aType isn't in the array already. >+ nsAutoString typeString, builtString; >+ PRUint16 parentType = 0; >+ >+ // we won't append xsd:anyType as the base of every type since that is kinda >+ // redundant. >+nsresult >+nsXFormsModelElement::WalkTypeChainInternal(nsISchemaType *aType, >+ PRUint16 *aBuiltinType, >+ nsStringArray *aTypeArray) >+{ >+ // if aBuiltinType is !nsnull, then we are only looking for the root >+ // primative type for aType. Otherwise build the whole string array. Sorry for that. Cannot pass it :). That's just nitpicking but it will looks better if first letter of sentence is capital. >+ if (!onlyFindBuiltinRoot) { >+ nsAutoString builtString; >+ rv = aType->GetTargetNamespace(builtString); >+ NS_ENSURE_SUCCESS(rv, rv); >+ nsAutoString typeName; >+ rv = aType->GetName(typeName); >+ NS_ENSURE_SUCCESS(rv, rv); >+ builtString.AppendLiteral("#"); >+ builtString.Append(typeName); >+ aTypeArray->AppendString(builtString); >+ } >+ >+ rv = WalkTypeChainInternal(simpleType, aBuiltinType, aTypeArray); >+ >+ return rv; >+ } The code duplication. I guess we can use it as for SCHEMA_TYPE_SIMPLE as for SCHEMA_TYPE_COMPLEX. >+NS_IMETHODIMP >+nsXFormsModelElement::GetDerivedTypeList(const nsAString &aType1, >+ const nsAString &aNamespace1, >+ nsAString &aBuiltinType) Again, I didn't get why you use aType1 instead aType. >+ nsCOMPtr<nsISchemaCollection> schemaColl = do_QueryInterface(mSchemas); >+ NS_ENSURE_STATE(schemaColl); >+ >+ nsCOMPtr<nsISchemaType> sType; Probably schemaType? > > nsBoundType > nsXFormsUploadElement::GetBoundType() Can we use constants from nsISchemaBuiltinType instead of nsBoundType? >+ >+ nsBoundType result = TYPE_DEFAULT; >+ switch (builtinType) { >+ case nsISchemaBuiltinType::BUILTIN_TYPE_BASE64BINARY: > result = TYPE_BASE64; >+ break; Probably it we don't need 'result' variable. For example: >+ switch (builtinType) { >+ case nsISchemaBuiltinType::BUILTIN_TYPE_BASE64BINARY: > return TYPE_BASE64; > >+html|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+html|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"], >+html|*:root secret[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+html|*:root secret[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"], >+html|*:root textarea[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+html|*:root textarea[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"], >+xul|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+xul|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"], >+xul|*:root secret[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+xul|*:root secret[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"], >+xul|*:root textarea[mozType|typelist~="http://www.w3.org/2001/XMLSchema#base64Binary"], >+xul|*:root textarea[mozType|typelist~="http://www.w3.org/2001/XMLSchema#hexBinary"] { >+ display: none; >+} >+ Do we throw warning if input and others are bound to instance node of wrong type like we do it for upload? The rest is fine for me.

(In reply to comment #43) > (From update of attachment 231876 [details] [diff] [review] [edit]) > > Do we throw warning if input and others are bound to instance node of wrong > type like we do it for upload? > Not yet. We'll have to do it for range, too. Problem with input is that there is no .cpp code to it so we'll either have to set up the refresh method in .cpp or put GetRootBuiltinType in an interface and do in input's XBL code. I'll get those two things done in the followup bugs for range and input. Other concerns taken care of in my next version of the patch.

Now nsXFormsUploadElement::GetBoundType() contain no any specific upload code. We can move it to nsXFormsDelegate or somewhere else. Probably model can have method GetRootBuiltinTypeForControl(), at least it will looks like GetTypeForControl() method.

Comment on attachment 232049 [details] [diff] [review] patch with comments fixed > [uuid(74238f22-d112-4bed-a8dc-cc228df52c2e)] > interface nsIModelElementPrivate : nsIXFormsModelElement Please update the uuid whenever you're changing an interface. > >+nsresult >+nsXFormsModelElement::AppendBuiltinTypes(PRUint16 aType, >+ nsStringArray *aTypeArray) nit, add one space before aType. ... >+ break; >+ default: >+ // should never hit here >+ return NS_ERROR_FAILURE; Could you add NS_WARNING here. >+ case nsISchemaSimpleType::SIMPLE_TYPE_UNION: >+ { >+ // XXX union types will screw us up. A union means that the type could >+ // be of any type listed in the union and still be valid. But we don't >+ // know which path it will take since we'd basically have to validate >+ // the node value to know. We'll have to figure out how to properly >+ // handle this later, though we may never need to. Waiting on a >+ // comeback from the WG to tell us if supporting this is necessary. >+ return NS_ERROR_XFORMS_UNION_TYPE; Is there a bug open for this? Please add the bug number somewhere here. >+nsXFormsModelElement::GetDerivedTypeList(const nsAString &aType, ... >+ nsStringArray *typeArray = new nsStringArray(); >+ if (!typeArray) { >+ return NS_ERROR_OUT_OF_MEMORY; Is it possible to create nsStringArray in stack? Would be safer if someone modifies this code later and adds some NS_ENSURE_XXXs.

(In reply to comment #47) > (From update of attachment 232049 [details] [diff] [review] [edit]) > >+ case nsISchemaSimpleType::SIMPLE_TYPE_UNION: > >+ { > >+ // XXX union types will screw us up. A union means that the type could > >+ // be of any type listed in the union and still be valid. But we don't > >+ // know which path it will take since we'd basically have to validate > >+ // the node value to know. We'll have to figure out how to properly > >+ // handle this later, though we may never need to. Waiting on a > >+ // comeback from the WG to tell us if supporting this is necessary. > >+ return NS_ERROR_XFORMS_UNION_TYPE; > Is there a bug open for this? Please add the bug number somewhere here. > > Heard back from John Boyer of the WG. He said that strictly interpreting the spec, we don't need to handle unions since we are just concerned about certain controls binding to certain types AND restictions of those types. If a processor does, then that is implementation dependent and not something that a form author should count on. So I'll remove the XXX and update the comment to say that we won't support union types.

(In reply to comment #46) > Now nsXFormsUploadElement::GetBoundType() contain no any specific upload code. > We can move it to nsXFormsDelegate or somewhere else. Probably model can have > method GetRootBuiltinTypeForControl(), at least it will looks like > GetTypeForControl() method. > It is absolutely moveable. But I don't think we should move it until some other control wants to use it. And I don't know if delegate is the place to put it since everything in delegate is meant to be accessible from the widgets and I don't know if this is the type of thing that widgets are looking for.

patch to checkin. Fixes Smaug's comments. Leaves GetBoundNode in upload for now. Will probably have a better understanding if this could be used or live elsewhere when we tackle xf:range.

(In reply to comment #50) > Created an attachment (id=232849) [edit] > patch to checkin > > patch to checkin. Fixes Smaug's comments. Leaves GetBoundNode in upload for > now. Will probably have a better understanding if this could be used or live > elsewhere when we tackle xf:range. > Ok, that's fine.

checked into the trunk

checked into 1.8.0 branch on 2006/09/21

checked into 1.8 branch on 2006/11/21