Submissions with "mailto:" action not working

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
12 years ago
a year ago

People

(Reporter: Stephen Pride, Assigned: Doron Rosenberg (IBM))

Tracking

({fixed1.8.0.2, fixed1.8.1})

Trunk
x86
All
fixed1.8.0.2, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

864 bytes, application/xhtml+xml
Details
5.13 KB, patch
aaronr
: review+
smaug
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
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
(Reporter)

Comment 1

12 years ago
Created attachment 190016 [details]
testcase
(Assignee)

Comment 2

12 years ago
<biesi> 1759     nsCOMPtr<nsIUploadChannel> uploadChannel =
do_QueryInterface(channel);
<biesi> 1760     NS_ENSURE_STATE(uploadChannel);
<biesi> that'll fail for mailto:
Assignee: aaronr → doronr
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

12 years ago
> 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 :-)
(Assignee)

Comment 4

12 years ago
(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

12 years ago
(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

12 years ago
> 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

12 years ago
(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.
(Assignee)

Comment 8

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

Comment 9

12 years ago
> 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

12 years ago
(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
(Assignee)

Comment 11

12 years ago
Created attachment 204737 [details] [diff] [review]
patch

This delegates the mailto: to the OS to handle.
Attachment #204737 - Flags: review?(aaronr)

Comment 12

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

Comment 13

12 years ago
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.
Attachment #204737 - Flags: review?(aaronr) → review-
(Assignee)

Comment 14

12 years ago
Created attachment 205860 [details] [diff] [review]
better patch

Better patch, also adds a warning in case the body param is already in the mailto submission uri.
Attachment #204737 - Attachment is obsolete: true
Attachment #205860 - Flags: review?(aaronr)

Comment 15

12 years ago
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.
Attachment #205860 - Flags: review?(aaronr) → review+
(Assignee)

Updated

12 years ago
Attachment #205860 - Flags: review?(smaug)

Comment 16

12 years ago
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
Attachment #205860 - Flags: review?(smaug) → review+
(Assignee)

Comment 17

12 years ago
checked into trunk
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Comment 18

12 years ago
checked into MOZILLA_1_8_BRANCH via bug 323691.  Reopening for now until it gets into 1.8.0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

12 years ago
Whiteboard: xf-to-branch

Updated

12 years ago
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Keywords: fixed1.8.0.2
Resolution: --- → FIXED

Comment 19

11 years ago
verified fixed in MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.