Schemas can define if attributed need to be qualified or not, which is important for schema validation.

Comment on attachment 195926 [details] [diff] [review] patch >Index: extensions/webservices/public/nsISchema.idl >=================================================================== >Index: extensions/webservices/schema/src/nsSchema.cpp >=================================================================== >+ if (attributeFormDefault.EqualsLiteral("qualified")) { >+ mAttributeFormDefaultQualified = PR_TRUE; >+ } else { >+ // default is unqualified >+ mAttributeFormDefaultQualified = PR_FALSE; >+ } I'd make that: mAttributeFormDefaultQualified = attributeFormDefault.EqualsLiteral("qualified"); I wonder whether we want to expose that as attributeFormQualified on nsISchemaAttribute. Wouldn't adding targetNamespace to nsISchemaAttribute be enough?

(In reply to comment #2) > (From update of attachment 195926 [details] [diff] [review] [edit]) > >Index: extensions/webservices/public/nsISchema.idl > >=================================================================== > >Index: extensions/webservices/schema/src/nsSchema.cpp > >=================================================================== > > >+ if (attributeFormDefault.EqualsLiteral("qualified")) { > >+ mAttributeFormDefaultQualified = PR_TRUE; > >+ } else { > >+ // default is unqualified > >+ mAttributeFormDefaultQualified = PR_FALSE; > >+ } > > I'd make that: > mAttributeFormDefaultQualified = > attributeFormDefault.EqualsLiteral("qualified"); > > I wonder whether we want to expose that as attributeFormQualified on > nsISchemaAttribute. Wouldn't adding targetNamespace to nsISchemaAttribute be > enough? > So if the attribute is not to be qualified, we would set an empty targetNamepace? Would increase the size of the object (string vs boolean), right? Would also require us to figure the right namespace on parsing of the schema. I think that would work fine too, not sure if we really care about the size increase?

I'm not talking about how to store the value (a boolean seems fine to me), but how to expose it. When getting the boolean value in this patch you always have access to the concrete classes, so there's no real need for it to be on the interface. You didn't add the code that actually uses this API, but it seems to me that when using this API a |readonly attribute AString targetNamespace| on nsISchemaAttribute would be more useful. nsSchemaAttribute::GetTargetNamespace would just be |if (mAttributeFormQualified && mSchema) return mSchema->GetTargetNamespace(aTargetNamespace); ...|.

Indeed, makes more sense your way. Here is the patch.

Comment on attachment 199092 [details] [diff] [review] patch >Index: extensions/webservices/public/nsISchema.idl >=================================================================== >+[scriptable, uuid(236ce1d7-213e-42e7-96ad-79f4cb3282a9)] > interface nsISchema : nsISchemaComponent { > /* Is this necessary? */ > readonly attribute AString schemaNamespace; >+ readonly attribute boolean attributeFormDefaultQualified; No need for this (you have IsAttributeFormDefaultQualified), unless you need this outside of schema code? >Index: extensions/webservices/schema/src/nsSchema.cpp >=================================================================== >@@ -58,20 +58,33 @@ nsSchema::nsSchema(nsISchemaCollection* >+ if (attributeFormDefault.EqualsLiteral("qualified")) { >+ mAttributeFormDefaultQualified = PR_TRUE; >+ } else { >+ // default is unqualified >+ mAttributeFormDefaultQualified = PR_FALSE; >+ } // default is unqualified mAttributeFormDefaultQualified = attributeFormDefault.EqualsLiteral("qualified"); >Index: extensions/webservices/schema/src/nsSchemaAttributes.cpp >=================================================================== >@@ -155,20 +155,32 @@ nsSchemaAttribute::GetFixedValue(nsAStri >+/* readonly attribute AString targetNamespace */ >+NS_IMETHODIMP >+nsSchemaAttribute::GetQualifiedNamespace(nsAString & aTargetNamespace) >+{ >+ if (mSchema && mAttributeFormQualified) >+ mSchema->GetTargetNamespace(aTargetNamespace); >+ else >+ aTargetNamespace.AssignLiteral(""); Truncate() >@@ -183,20 +195,28 @@ nsSchemaAttribute::SetConstraints(const >+NS_IMETHODIMP Just nsresult >+nsSchemaAttribute::SetAttributeFormQualified(PRBool aAttributeFormQualified) >@@ -316,38 +336,58 @@ nsSchemaAttributeRef::GetFixedValue(nsAS >+NS_IMETHODIMP >+nsSchemaAttributeRef::GetQualifiedNamespace(nsAString & aTargetNamespace) >+{ >+ if (mSchema && mAttributeFormQualified) >+ mSchema->GetTargetNamespace(aTargetNamespace); >+ else >+ aTargetNamespace.AssignLiteral(""); Truncate() >+NS_IMETHODIMP Just nsresult >+nsSchemaAttributeRef::SetAttributeFormQualified(PRBool aAttributeFormQualified) >Index: extensions/webservices/schema/src/nsSchemaLoader.cpp >=================================================================== >@@ -2784,57 +2784,82 @@ nsSchemaLoader::ProcessAttributeComponen >+ // set the qualified form >+ if (formValue.EqualsLiteral("qualified")) { >+ attributeRef->SetAttributeFormQualified(PR_TRUE); >+ } else if (formValue.EqualsLiteral("unqualified")) { I think the style in these files is: } else { >+ attributeRef->SetAttributeFormQualified(PR_FALSE); >+ } else { } else { >+ // get default >+ PRBool defaultvalue; >+ aSchema->GetAttributeFormDefaultQualified(&defaultvalue); PRBool defaultvalue = aSchema->IsAttributeFormDefaultQualified(); >+ // set the qualified form >+ if (formValue.EqualsLiteral("qualified")) { >+ attributeInst->SetAttributeFormQualified(PR_TRUE); >+ } else if (formValue.EqualsLiteral("unqualified")) { } else if { >+ attributeInst->SetAttributeFormQualified(PR_FALSE); >+ } else { } else { >+ // get default >+ PRBool defaultvalue; >+ aSchema->GetAttributeFormDefaultQualified(&defaultvalue); PRBool defaultvalue = aSchema->IsAttributeFormDefaultQualified(); >Index: extensions/webservices/schema/src/nsSchemaPrivate.h >=================================================================== >+ PRBool IsAttributeFormDefaultQualified() { return mAttributeFormDefaultQualified; } Make it |PRBool IsAttributeFormDefaultQualified() const|. >@@ -428,52 +430,56 @@ public: >+ NS_IMETHOD SetAttributeFormQualified(PRBool aAttributeFormQualified); Just nsresult. >+ NS_IMETHOD SetAttributeFormQualified(PRBool aAttributeFormQualified); Just nsresult.

checked in. Probably want this for the 1.8 branch for xforms.

mkaply - would this be allowed into 1.8.x?

Yes, assuming this isn't built by default.

Actually, this is built by default, part of webservices.

request approval. Give a really good reason :)

Comment on attachment 199092 [details] [diff] [review] patch XForms will need this, and since 1.8.x will be the next release, would be nice to get this. Low risk, the code itself is only used by webservices in release builds and won't break anything.

Comment on attachment 199092 [details] [diff] [review] patch Cannot change existing XPCOM interfaces on the 1.8 branch.

removing xf-to-branch since it sounds like this will never be allowed into branch.