301571 - Submissions with "mailto:" action not working

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Stephen Pride]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050721 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050721 Firefox/1.0+

Not sure if this is a high priority right now.  Using an <xf:submission> with an
action="mailto:xxxxx" does not work.  Testcase will be attached.

Reproducible: Always

Comment 1

[Stephen Pride]

Attachment: testcase

Comment 2

[Doron Rosenberg (IBM)]

<biesi> 1759     nsCOMPtr<nsIUploadChannel> uploadChannel =
do_QueryInterface(channel);
<biesi> 1760     NS_ENSURE_STATE(uploadChannel);
<biesi> that'll fail for mailto:

Comment 3

[Pascal Sartoretti]

> Not sure if this is a high priority right now. 

I am disappointed to see that this bug is not judged as important. Our company is very interested in building "low tech" workflow systems heavily based on XForms and e-mails. Any possibility to have this bug re-activated? 

I know, I could also stop complaining and start learning how to do Mozilla development :-)

Comment 4

[Doron Rosenberg (IBM)]

(In reply to comment #3)
> > Not sure if this is a high priority right now. 
> 
> I am disappointed to see that this bug is not judged as important. Our company
> is very interested in building "low tech" workflow systems heavily based on
> XForms and e-mails. Any possibility to have this bug re-activated? 
> 
> I know, I could also stop complaining and start learning how to do Mozilla
> development :-)
> 

The problem I have is that say Firefox doesn't come with mail support I believe.  So we would probably have the code open the default mail client of the system with the instance data in the body and have the user press the send button.

Is that ok, or do you want the mail to be sent without user interaction?

Comment 5

[aaronr]

(In reply to comment #4)
> (In reply to comment #3)
> > > Not sure if this is a high priority right now. 
> > 
> > I am disappointed to see that this bug is not judged as important. Our company
> > is very interested in building "low tech" workflow systems heavily based on
> > XForms and e-mails. Any possibility to have this bug re-activated? 
> > 
> > I know, I could also stop complaining and start learning how to do Mozilla
> > development :-)
> > 
> 
> The problem I have is that say Firefox doesn't come with mail support I
> believe.  So we would probably have the code open the default mail client of
> the system with the instance data in the body and have the user press the send
> button.
> 
> Is that ok, or do you want the mail to be sent without user interaction?
> 


If I had to guess, I'd think that Pascal would want the latter.  Just like any other submission, should be done in the background unless the author provides confirmation UI, etc. for it.

Pascal, can you add a testcase and describe exactly how you think that it should work?  Like this (http://support.microsoft.com/?kbid=279460), perhaps?

Comment 6

[Pascal Sartoretti]

> The problem I have is that say Firefox doesn't come with mail support I
> believe.  So we would probably have the code open the default mail client of
> the system with the instance data in the body and have the user press the send
> button.
> 
> Is that ok, or do you want the mail to be sent without user interaction?

I think that the user should not be concerned with URI scheme with which his form is submitted (http, https or mailto); ideally (and according to the XForms specs?), it should be done in the background, without user interaction.

However, as I understand that the implementation is not straightforward, it would be nice to have a quick workaround where the default e-mail client is opened, and the user has to press "Send".

Comment 7

[Allan Beaufour]

(In reply to comment #6)
> I think that the user should not be concerned with URI scheme with which his
> form is submitted (http, https or mailto); ideally (and according to the XForms
> specs?), it should be done in the background, without user interaction.

Argh no! I could use your mail client to spam people by doing an xform with mailto://

We might open up for the possibility for whitelisting domains, like we do for submissions in general, but the dfault should _definately_ ask the user for confirmation.

Comment 8

[Doron Rosenberg (IBM)]

You could always have a proxy script on a server that gets submitted to and then sends the mail :)

Comment 9

[Pascal Sartoretti]

> Argh no! I could use your mail client to spam people by doing an 
> xform with mailto://

You are definitely right. And besides, it seems logical that a "mailto:" URI has the same behaviour in XForms as in a regular HTML page: fire up the default mail client with the instance data in the body.

Comment 10

[Hermann Schwab]

(In reply to comment #9)
 
> it seems logical that a "mailto:" URI
> has the same behaviour in XForms as in a regular HTML page: fire up the default
> mail client with the instance data in the body.

Bug 316372 mailto: links do not open Compose window anymore

Comment 11

[Doron Rosenberg (IBM)]

Attachment: patch

This delegates the mailto: to the OS to handle.

Comment 12

[Allan Beaufour]

Can you not set the body parameter through nsURI or something?

Comment 13

[aaronr]

Comment on attachment 204737 [details] [diff] [review]
patch

>@@ -1823,20 +1825,90 @@ nsXFormsSubmissionElement::SendData(cons
...
>+      if (hasExposedMailClient) {
>+        nsCAutoString mailtoUrl;
>+        mailtoUrl.Assign(uriSpec);

can't you just pass in uriSpec to the nsCAutoString constructor?

>+
>+        // now add the body
>+        // we have to figure if body is the first url option (?) or an additional
>+        // one (&).  If it contains ?, then we use &
>+

maybe make this comment a little clearer?  I had to look up what a mailto string should look like to understand it.  Maybe something like, "We need to add the body as a parameter to the mailto uri.  The first parameter needs to be prepended by a '?' and all additional parameters need to be prepended by a '&'.  So figure out if our body parameter will be the first one in the string or just additional to the parameters already there."

>+        if (mailtoUrl.FindChar('?') != kNotFound)
>+          mailtoUrl.AppendLiteral("&body=");
>+        else
>+          mailtoUrl.AppendLiteral("?body=");
>+

What if there is already a body parameter in the string?  What happens?  Is it an error?  Or does it just ignore the extra ones and use the first/last?  Should we report an error if we encounter this problem?

>+        // get the stream contents
>+        PRUint32 len, read, numReadIn = 1;
>+        stream->Available(&len);
>+        char *buf = new char[len+1];
>+        memset(buf, 0, len+1);
>+
>+        // Read returns 0 if eos
>+        while (numReadIn != 0) {
>+          numReadIn = stream->Read(buf, len, &read);
>+          NS_EscapeURL(buf, esc_AlwaysCopy, numReadIn, mailtoUrl);
>+        }

Doesn't look right.  Maybe should be using 'read' instead of 'numReadIn' to NS_EscapeURL.  Otherwise you'll run into problems, I think, when numReadIn isn't 0 the first time and you have to go through the loop again.

>+
>+        delete [] buf;
>+
>+        // create an nsIUri out of the string
>+        nsCOMPtr<nsIURI> mailUri;
>+        ios->NewURI(mailtoUrl,
>+                    nsnull,
>+                    nsnull,
>+                    getter_AddRefs(mailUri));
>+        NS_ENSURE_STATE(mailUri);

Here and other places below: all of these NS_ENSURE_xxx's will exit this function.  Shouldn't EndSubmit() be called before this function exits?

>+
>+        // let the OS handle the uri
>+        rv = extProtService->LoadURI(mailUri, nsnull);
>+
>+        if (NS_FAILED(rv)) {
>+          // opening an mail client failed.
>+          nsXFormsUtils::ReportError(NS_LITERAL_STRING("submitMailtoFailed"),
>+                                     mElement);
>+        }
>+
>+      } else {
>+        // no system mail client found
>+        nsXFormsUtils::ReportError(NS_LITERAL_STRING("submitMailtoInit"),
>+                                   mElement);

shouldn't this be out a level?  Or if not, should probably give some kind of message to the console if we encountered a submission that we don't understand (i.e. not replace instance and not mailto).  Because it looks like we do nothing in that case.

>+      }
>+    }
>+
>+    EndSubmit(PR_TRUE);
>+    return NS_OK;
>+  }
>+
>   if (!CheckSameOrigin(doc->GetDocumentURI(), uri)) {
>     nsXFormsUtils::ReportError(NS_LITERAL_STRING("submitSendOrigin"),
>                                mElement);
>     return NS_ERROR_ABORT;
>   }
> 
>   // wrap the entire upload stream in a buffered input stream, so that
>   // it can be read in large chunks.
>   // XXX necko should probably do this (or something like this) for us.
>   nsCOMPtr<nsIInputStream> bufferedStream;
>Index: extensions/xforms/resources/locale/en-US/xforms.properties
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/locale/en-US/xforms.properties,v
>retrieving revision 1.16
>diff -w -u -p -1 -0 -r1.16 xforms.properties
>--- extensions/xforms/resources/locale/en-US/xforms.properties	17 Nov 2005 22:00:27 -0000	1.16
>+++ extensions/xforms/resources/locale/en-US/xforms.properties	1 Dec 2005 22:18:05 -0000
>@@ -58,17 +58,19 @@ labelLink2Error      = XForms Error (19)
> invalidSeparator     = XForms Error (20): Submission separator may only be either "&" or ";", but found "%S".
> instanceBindError    = XForms Error (21): Submission failed trying to replace instance document '%S'.  Instance document doesn't exist in same model as submission element.
> instanceInstanceLoad = XForms Error (22): Instance document not allowed to load external instance: %S
> rangeSetSliderNaN    = XForms Error (23): NaN passed to setSlider()
> rangeNullObjects     = XForms Error (24): One or more of the passed objects were null
> rangeNullInit        = XForms Error (25): One or more init() parameters is NaN
> rangeBeginEndError   = XForms Error (26): Begin is higher than end?
> encodingMemoryError  = XForms Error (23): Not enough available memory to encode file %S, size = %S.
> uploadBoundTypeError = XForms Error (24): Upload element not bound to valid datatype.  Must be bound to datatype 'xsd:anyURI', 'xsd:base64Binary', or 'xsd:hexBinary'.
> copyError            = XForms Error (25): A copy element was found whose parent is not an itemset element
>+submitMailtoInit     = XForms Error (26): No mailto: handler found
>+submitMailtoFailed   = XForms Error (27): Failed to load a mail client for an mailto: submission

nit: should be 'a' not 'an' in submitMailtoFailed.

Comment 14

[Doron Rosenberg (IBM)]

Attachment: better patch

Better patch, also adds a warning in case the body param is already in the mailto submission uri.

Comment 15

[aaronr]

Comment on attachment 205860 [details] [diff] [review]
better patch

I verified that we only need to call EndSubmit if we return NS_OK from ::SendData.  Rest of my issues were addressed with this patch.  My only nit is that EndSubmit is called in 3 diff. places instead of just once using a flag that you set in those other places instead.  But it isn't that big of a deal.  As long as EndSubmit is the last thing done before the return NS_OK.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 205860 [details] [diff] [review]
better patch


>+        // get the stream contents
>+        PRUint32 len, read, numReadIn = 1;
>+        stream->Available(&len);

Check the return code of the Available()


>+        char *buf = new char[len+1];

Handle OOM


With those r=me

Comment 17

[Doron Rosenberg (IBM)]

checked into trunk

Comment 18

[aaronr]

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

Comment 19

[aaronr]

verified fixed in MOZILLA_1_8_BRANCH