serialization using method="multipart-post" doesn't serialize parts
Categories
(Core Graveyard :: XForms, defect)
Tracking
(Not tracked)
People
(Reporter: sspeiche, Assigned: msterlin)
References
()
Details
(Keywords: fixed1.8.0.12, fixed1.8.1.4)
Attachments
(4 files, 2 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060911 Minefield/3.0a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060911 Minefield/3.0a1 Files marked by the <upload> control as xsd:anyURI and with submission method="multipart-post", the referenced resources are to be serialized as multipart/related. The resources referenced by upload control are not being serialized. Reproducible: Always
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Comment 2•18 years ago
|
||
Here's what gets submitted with Moz: -----------------------------24464570528145 Content-Type: application/xml Content-ID: <5af1.41bb@mozilla.org> <?xml version="1.0" encoding="UTF-8"?> <mail xmlns:xhtml="http://www.w3.org/1999/xhtml" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xforms="http://www.w3.org/2002/xforms"> <picture> <attach xsi:type="xsd:anyURI">file:///C:/temp/crap2.xml</attach> <name xsi:type="xsd:anyURI"/> <type xsi:type="xsd:string">text/xml</type> </picture> </mail> -----------------------------24464570528145 Content-Type: application/octet-stream Content-Transfer-Encoding: binary Content-ID: <26e9.1eb@mozilla.org> -----------------------------24464570528145 Content-Type: application/octet-stream Content-Transfer-Encoding: binary Content-ID: <bb3.2ea6@mozilla.org> -----------------------------24464570528145-- Here's what gets submitted with X-Smiles: --------xs1q2w3e4r5t6y7u8i50180 Content-Type: application/xml Content-ID: <content-start35771@xsmiles.org> <?xml version="1.0" encoding="ISO-8859-1"?> <mail xmlns:xforms="http://www.w3.org/2002/xforms" xmlns:xhtml="http://www.w3.org/1999/xhtml" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> <picture> <attach xsi:type="xsd:anyURI">cid:X432584@xsmiles.org</attach> <name xsi:type="xsd:anyURI"/> <type xsi:type="xsd:string">application/xml</type> </picture> </mail> --------xs1q2w3e4r5t6y7u8i50180 Content-Type: application/xml Content-Transfer-Encoding: binary Content-ID: <cid:X432584@xsmiles.org> <?xml version="1.0" encoding="UTF-8"?> <db:car xmlns:db="http://www.example.org/double" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.example.org/double crap2.xsd "> <db:price>-INF</db:price> </db:car> --------xs1q2w3e4r5t6y7u8i50180
Assignee | ||
Comment 3•18 years ago
|
||
TCPMon: listen port = 8081; targetHostname = www.xformstest.org; targetPort=80 xforms:submission action=http://localhost:8081/cgi-bin/echo.sh or showinstance.sh
Assignee | ||
Comment 4•18 years ago
|
||
Assignee | ||
Comment 5•18 years ago
|
||
nsXFormsUploadElement sets the UploadFileProperty to a pointer to a LocalFile but when the submission document is created in nsFormsSubmissionElement, ImportNode does not copy the property so we think there is no file to upload. The patch will check if the property exists in the original node and will clone the file and set the property on the new node in the submission doc. Additionally, the file URI that was put in the node by upload element needs to be replaced with the generated content ID in createAttachments: currentNode->setNodeValue(cid) doesn't work. If we get the first child and set its node value to the content ID, the submitted document is correct. If this patch is to be checked in before January, any review comments will have to be handled by someone else as I will be out for the rest of the year; otherwise I'll address any issues when I return.
Comment on attachment 247911 [details] [diff] [review] patch I've got to r- because this patch won't handle the case where the upload is bound to an attribute node. This patch assumes the data node is an element node and it shouldn't. I'll fix up the patch since Merle is on vacation.
testcase where upload is bound to an attribute node in the instance data.
Fixed up Merle's patch to properly handle when upload binds to attributes.
Comment 9•18 years ago
|
||
Comment on attachment 248189 [details] [diff] [review] patch2 Possibly too late here to review :) , but looks ok to me, especially if you add some comment why the following isn't needed: - } else { - nsCOMPtr<nsIAttribute> attr = do_QueryInterface(currentNode); - NS_ENSURE_STATE(attr); - uploadFileProperty = - attr->GetProperty(nsXFormsAtoms::uploadFileProperty); }
Comment 10•18 years ago
|
||
adds attribute comment per smaug's review comment.
Comment 11•18 years ago
|
||
Comment on attachment 248195 [details] [diff] [review] patch3 > nsresult > nsXFormsSubmissionElement::CreateAttachments(nsIModelElementPrivate *aModel, > nsIDOMNode *aNode, > SubmissionAttachmentArray *aAttachments) Code of CreateAttachments looks very intricated though idea of the method is very simple. I guess you can use here document traversal api to simplify things and to avoid code dublication.
Comment 12•18 years ago
|
||
Comment on attachment 248195 [details] [diff] [review] patch3 >+ // the cloning does not copy any properties of the currentNode. If >+ // the attribute node has an uploadFileProperty we need to copy it >+ // to the submission document so that local files will be attached >+ // properly when the submission format is multipart-related. .... I don't like very big methods. Therefore can you move this code into new method? Also I guess this allow to avoid code dublication.
Comment 13•18 years ago
|
||
(In reply to comment #12) > (From update of attachment 248195 [details] [diff] [review] [edit]) > > >+ // the cloning does not copy any properties of the currentNode. If > >+ // the attribute node has an uploadFileProperty we need to copy it > >+ // to the submission document so that local files will be attached > >+ // properly when the submission format is multipart-related. > > .... > > I don't like very big methods. Therefore can you move this code into new > method? Also I guess this allow to avoid code dublication. > I can move the code into a different method. But it won't be any smaller. I don't see any duplication that can be avoided. nsIContent and nsIAttribute are distinctly different and they each have their own Set/GetProperty methods on the branches though I believe on trunk the property stuff was moved up a level. Do you still want the code moved out to another function?
Comment 14•18 years ago
|
||
(In reply to comment #11) > (From update of attachment 248195 [details] [diff] [review] [edit]) > > > nsresult > > nsXFormsSubmissionElement::CreateAttachments(nsIModelElementPrivate *aModel, > > nsIDOMNode *aNode, > > SubmissionAttachmentArray *aAttachments) > > Code of CreateAttachments looks very intricated though idea of the method is > very simple. I guess you can use here document traversal api to simplify things > and to avoid code dublication. > ? Can you give me an example of what you mean?
Comment 15•18 years ago
|
||
(In reply to comment #14) > > Code of CreateAttachments looks very intricated though idea of the method is > > very simple. I guess you can use here document traversal api to simplify things > > and to avoid code dublication. > > > > ? Can you give me an example of what you mean? > I meant http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/, partially http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/nsIDOMDocumentTraversal.idl. DOM elements as well DOM attributes is be traversed. NodeFilter::accept() will allow only those nodes that have uploadFileProperty. nsIDOMTreeWalker methods will be used to process nodes and to add attachment. Or something else. The result is you have one 'for' statement instead recursive createAttachments() calls and traversing in cycle through attributes.
Comment 16•18 years ago
|
||
(In reply to comment #13) > Do you still want the code moved out to another function? > Yes, I like :). The method CloneUploadProperty() or something else helps to keep code more readable. > I can move the code into a different method. But it won't be any smaller. I > don't see any duplication that can be avoided. nsIContent and nsIAttribute are > distinctly different and they each have their own Set/GetProperty methods on > the branches though I believe on trunk the property stuff was moved up a level. So, probably it makes sence to have one more method to get/set property. Right, nsiContent and nsIAttribute is different things but following code is common: // Clone the local file so the same pointer isn't released twice. nsIFile *file = NS_STATIC_CAST(nsIFile *, uploadFileProperty); nsIFile *fileCopy = nsnull; nsresult rv = file->Clone(&fileCopy); NS_ENSURE_SUCCESS(rv, rv); destChildContent->SetProperty(nsXFormsAtoms::uploadFileProperty, fileCopy, ReleaseObject); So, if you can have method like CloneUploadPropertyFrom(nsIDOMNode* aSource, nsIDOMNode* aDest) that will handle both attribute node and element node. But probably it doesn't make sence since dublication is little. It's up to you.
Comment 17•17 years ago
|
||
(In reply to comment #15) > (In reply to comment #14) > > > > Code of CreateAttachments looks very intricated though idea of the method is > > > very simple. I guess you can use here document traversal api to simplify things > > > and to avoid code dublication. > > > > > > > ? Can you give me an example of what you mean? > > > > I meant http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/, > partially > http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/nsIDOMDocumentTraversal.idl. > DOM elements as well DOM attributes is be traversed. NodeFilter::accept() will > allow only those nodes that have uploadFileProperty. nsIDOMTreeWalker methods > will be used to process nodes and to add attachment. Or something else. The > result is you have one 'for' statement instead recursive createAttachments() > calls and traversing in cycle through attributes. > Looking at the spec (http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/DOM2-Traversal-Range.txt), it doesn't look like using a nsIDOMTreeWalker isn't going to buy us much. It says: SHOW_ATTRIBUTE Show Attr nodes. This is meaningful only when creating an iterator or tree-walker with an attribute node as its root; in this case, it means that the attribute node will appear in the first position of the iteration or traversal. Since attributes are never children of other nodes, they do not appear when traversing over the document tree. So the dom tree walker won't encounter attributes unless we set the root of the tree walker to be an attribute. So we couldn't filter out any element nodes just because they didn't have the upload property since either way we'd still have to check each element for attributes by hand like we are currently doing.
Comment 18•17 years ago
|
||
(In reply to comment #16) > (In reply to comment #13) > > > Do you still want the code moved out to another function? > > > > Yes, I like :). The method CloneUploadProperty() or something else helps to > keep code more readable. > > > I can move the code into a different method. But it won't be any smaller. I > > don't see any duplication that can be avoided. nsIContent and nsIAttribute are > > distinctly different and they each have their own Set/GetProperty methods on > > the branches though I believe on trunk the property stuff was moved up a level. > > So, probably it makes sence to have one more method to get/set property. Right, > nsiContent and nsIAttribute is different things but following code is common: > > // Clone the local file so the same pointer isn't released twice. > nsIFile *file = NS_STATIC_CAST(nsIFile *, uploadFileProperty); > nsIFile *fileCopy = nsnull; > nsresult rv = file->Clone(&fileCopy); > NS_ENSURE_SUCCESS(rv, rv); > destChildContent->SetProperty(nsXFormsAtoms::uploadFileProperty, > fileCopy, > ReleaseObject); > > So, if you can have method like > CloneUploadPropertyFrom(nsIDOMNode* aSource, nsIDOMNode* aDest) that will > handle both attribute node and element node. > > But probably it doesn't make sence since dublication is little. It's up to you. > I'd vote no. Not much in way of savings. I'll keep it in mind in case we set the property somewhere else in the future. 3 times would probably be worth a utility function.
Comment 19•17 years ago
|
||
Comment on attachment 248195 [details] [diff] [review] patch3 In lieu of my last two comments, does this pass the muster?
Comment 20•17 years ago
|
||
Comment on attachment 248195 [details] [diff] [review] patch3 r=me, sorry about traversal :)
Comment 21•17 years ago
|
||
checked into trunk for msterlin
Comment 22•17 years ago
|
||
checked into 1.8 branch on 2007-04-12 checked into 1.8.0 branch on 2007-04-16
Updated•8 years ago
|
Comment 23•2 years ago
|
||
(In reply to aaronr from comment #17)
(In reply to comment #15)
(In reply to comment #14)
Code of CreateAttachments looks very intricated though idea of the method is
very simple. I guess you can use here document traversal api to simplify things
and to avoid code dublication.? Can you give me an example of what you mean?
I meant http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/,
partially
http://lxr.mozilla.org/mozilla/source/dom/public/idl/traversal/nsIDOMDocumentTraversal.idl.
DOM elements as well DOM attributes is be traversed. NodeFilter::accept() will
allow only those nodes that have uploadFileProperty. nsIDOMTreeWalker methods
will be used to process nodes and to add attachment. Or something else. The
result is you have one 'for' statement instead recursive createAttachments()
calls and traversing in cycle through attributes.https://hotmyfreecams.com/
Looking at the spec
(http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/DOM2-
Traversal-Range.txt), it doesn't look like using a nsIDOMTreeWalker isn't
going to buy us much. It says:SHOW_ATTRIBUTE
Show Attr nodes. This is meaningful only when creating
an iterator or tree-walker with an attribute node as its
root; in this case, it means that the attribute node
will appear in the first position of the iteration or
traversal. Since attributes are never children of other
nodes, they do not appear when traversing over the
document tree.
https://bestpornsites.mobi
So the dom tree walker won't encounter attributes unless we set the root of
the tree walker to be an attribute. So we couldn't filter out any element
nodes just because they didn't have the upload property since either way
we'd still have to check each element for attributes by hand like we are
currently doing.
ty
Description
•