This bug is for fixing some of the file submission issues I noticed when working on bug 275453.

- I changed |ParseTypeFromNode()| to return the the actual namespace URI rather than just the textual prefix. This is more useful, and all of the code that currently uses this call is doing the conversion from prefix to URI anyway. - In |nsXFormsSubmissionElement::CopyChildren()|, the code was never going down the path to add upload the contents of a local file. This is because it was calling |GetElementEncodingType()| on 'destChild', which is a clone of 'currentNode', but doesn't contain the relevant attributes. Passing 'currentNode' fixed this issue. - In |nsXFormsSubmissionElement::SerializeDataMultipartRelated()|, we were never checking the return value for |SerializeDataXML()|. So if that failed, we would still try to submit, and crash. - Some of the code incorrectly referenced 'base64binary', when we should be using 'base64Binary' (noticed capitalized 'B').

Any testcase?

I've only got a test case that works with the upload patch. So this can wait until that gets checked in.

Comment on attachment 196958 [details] [diff] [review] patch >Index: nsXFormsSubmissionElement.cpp >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsSubmissionElement.cpp,v >retrieving revision 1.40 >diff -u -6 -p -r1.40 nsXFormsSubmissionElement.cpp >--- nsXFormsSubmissionElement.cpp 15 Sep 2005 11:37:37 -0000 1.40 >+++ nsXFormsSubmissionElement.cpp 21 Sep 2005 22:19:47 -0000 >@@ -1227,13 +1223,13 @@ nsXFormsSubmissionElement::CopyChildren( > // an attachments array, then we need to perform multipart/related > // processing (i.e., generate a ContentID for the child of this element, > // and append a new attachment to the attachments array). > > PRUint32 encType; > if (attachments && >- NS_SUCCEEDED(GetElementEncodingType(destChild, &encType)) && >+ NS_SUCCEEDED(GetElementEncodingType(currentNode, &encType)) && > encType == ELEMENT_ENCTYPE_URI) nit: You should probably change the comment above this change to say currentNode instead of destChild, right? >Index: nsXFormsUtils.h >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUtils.h,v >retrieving revision 1.30 >diff -u -6 -p -r1.30 nsXFormsUtils.h >--- nsXFormsUtils.h 23 Aug 2005 11:00:50 -0000 1.30 >+++ nsXFormsUtils.h 21 Sep 2005 22:19:47 -0000 >@@ -368,18 +368,20 @@ public: > */ > static NS_HIDDEN_(nsresult) GetInstanceNodeForData(nsIDOMNode *aInstanceDataNode, > nsIModelElementPrivate *aModel, > nsIDOMNode **aInstanceNode); > > /** >- * This function takes an instance data node, finds the type bound to it, and >- * returns the seperated out type (integer) and namespace prefix (xsd). >+ * This function takes an node from the main document and an instance data >+ * node, finds the type bound to it, and returns the seperated out type and >+ * namespace URI. > */ Please seperate out the individual parameters to comment on each like the other functions below it in the header file. Might also want to spell out that aElement can be ANY element from the XForms document and that it is used to resolve the NS prefix to the proper NS URI as defined in the XForms document. Otherwise it isn't really clear what the caller should pass in for aElement. With that, r=me

Patch with Aaron's suggested fixes.

(In reply to comment #3) > I've only got a test case that works with the upload patch. So this can wait > until that gets checked in. Ok, I'll wait then because I'd rather see a testcase first.

To recreate submission attachment error: 1) Click Browse button and select a file. 2) Click Submit button. => This is a multipart-post, so the page that loads should show the instance data, followed by the file you selected. The path name in the <attachment> node should be replaced by an ID string, which points to the attached file. To recreate crash: 1) Click Browse button and select a file. 2) Click Clear. 3) Click Submit. => Should fail to submit, since the <attachment> node doesn't validate. Instead, it tries to submit and crashes.

No new functionality/fixes. Just updated to work better with latest patch for bug 275453.

Comment on attachment 202267 [details] [diff] [review] patch v1.3 r=me if you open a new bug about not being able to upload and submit big files using the attached testcase.

Comment on attachment 202267 [details] [diff] [review] patch v1.3 > /* static */ nsresult >-nsXFormsUtils::ParseTypeFromNode(nsIDOMNode *aInstanceData, >- nsAString &aType, nsAString &aNSPrefix) >+nsXFormsUtils::ParseTypeFromNode(nsIDOMElement* aElement, >+ nsIDOMNode *aInstanceData, >+ nsAString &aType, nsAString &aNSUri) > { > nsresult rv = NS_OK; > > // aInstanceData could be an instance data node or it could be an attribute > // on an instance data node (basically the node that a control is bound to). > >@@ -1271,28 +1272,39 @@ nsXFormsUtils::ParseTypeFromNode(nsIDOMN > > if (NS_FAILED(rv) || !typeVal) { > return NS_ERROR_NOT_AVAILABLE; > } > > // split type (ns:type) into namespace and type. >+ nsAutoString prefix; > PRInt32 separator = typeVal->FindChar(':'); > if ((PRUint32) separator == (typeVal->Length() - 1)) { > const PRUnichar *strings[] = { typeVal->get() }; > // XXX: get an element from the document this came from > ReportError(NS_LITERAL_STRING("missingTypeName"), strings, 1, nsnull, nsnull); > return NS_ERROR_UNEXPECTED; > } else if (separator == kNotFound) { > // no namespace prefix, which is valid; >- aNSPrefix.AssignLiteral(""); >+ prefix.AssignLiteral(""); > aType.Assign(*typeVal); > } else { >- aNSPrefix.Assign(Substring(*typeVal, 0, separator)); >+ prefix.Assign(Substring(*typeVal, 0, separator)); > aType.Assign(Substring(*typeVal, ++separator, typeVal->Length())); > } > >- return NS_OK; >+ if (prefix.IsEmpty()) { >+ aNSUri.AssignLiteral(""); >+ } else { >+ // get the namespace url from the prefix >+ nsCOMPtr<nsIDOM3Node> domNode3 = do_QueryInterface(aElement, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ rv = domNode3->LookupNamespaceURI(prefix, aNSUri); >+ } >+ >+ return rv; > } This is (still) wrong, I think. The namespace lookup should happen first on aInstanceData node and if not found there, then using the <xf:instance> element (which can be get using GetInstanceNodeForData()).

(In reply to comment #10) > (From update of attachment 202267 [details] [diff] [review] [edit]) > ...not being able to upload and submit big files > using the attached testcase. > Any idea what is causing this problem?

Comment on attachment 202412 [details] [diff] [review] patch v1.4 How's this, smaug?

Comment on attachment 202412 [details] [diff] [review] patch v1.4 >- return NS_OK; >+ if (prefix.IsEmpty()) { >+ aNSUri.AssignLiteral(""); >+ } else { >+ // get the namespace url from the prefix using instance data node >+ nsCOMPtr<nsIDOM3Node> domNode3 = do_QueryInterface(aInstanceData, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ rv = domNode3->LookupNamespaceURI(prefix, aNSUri); >+ >+ if (NS_FAILED(rv) || aNSUri.Length() == 0) { Hmm, domNode3->LookupNamespaceURI never fails, but returns null if namespace was not found. So I'd use: if (DOMStringIsNull(aNSUri)) (if that works ;) ) r=me

(In reply to comment #15) > (From update of attachment 202412 [details] [diff] [review] [edit]) > Hmm, domNode3->LookupNamespaceURI never fails, but returns null if > namespace was not found. So I'd use: > > if (DOMStringIsNull(aNSUri)) > Sorry, nsDOMAttribute may return an error. So: if (NS_FAILED(rv) || DOMStringIsNull(aNSUri))

(In reply to comment #16) > (In reply to comment #15) > Sorry, nsDOMAttribute may return an error. So: > if (NS_FAILED(rv) || DOMStringIsNull(aNSUri)) > Ugh, idiot me. nsDOMAttribute never returns an error. if (DOMStringIsNull(aNSUri)) should be enough!

Checked in to trunk with smaug's change. -> FIXED Opened bug 315767 for large-file-submission issue.

checked into MOZILLA_1_8_BRANCH via bug 323691. Reopening for now until it gets into 1.8.0

verified fixed in MOZILLA_1_8_BRANCH