Closed Bug 191248 (mailto_broken) Opened 22 years ago Closed 22 years ago

mailto: Form submission broken

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: Matti, Assigned: d_king)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

This is a regression between 1.2.1 and 1.3a 1) load URL 2) file something in the forms 3) submit 4) get a compose window only prefilled with the "TO:" field. I'll try to find the excat regression date in the next days
This seems to be working in 2002-11-19-08 but not in 2002-11-20-08 There's a whole slew of checkins in there, but none look obviously responsible....
OS: Windows 2000 → All
Hardware: PC → All
Severity: normal → major
Keywords: regression
Nominating as 1.3beta blocker...
Flags: blocking1.3b?
seems like a good one to get for beta. Who can own this?
Flags: blocking1.3b? → blocking1.3b+
Asa Dotzler wrote: > seems like a good one to get for beta. Who can own this? jkeiser did the main work in bug 61893 ("Form data not being mailed when using mailto: in form action attribute (eg <form action=mailto:user@company.com> [form sub]") - so I guess he is the ultimative expert for this code... :)
Assignee: form-submission → jkeiser
Alias: mailto_broken
Looks like the fix for bug 176904 was responsible. The problem is that we no longer get the string representation of the mailto url... All we do is call SetPath on it. Too bad nsMailtoUrl::SetPath() never calls ParseUrl() (unlike nsMailtoUrl::SetSpec). In fact, most of the setters there have this bug (it's all original vanilla mscott code, btw, so no one to blame offhand... ;) ).
bz: Are you saying that the fix for this would be to add ParseUrl() to nsMailtoUrl::SetPath() ?
Yes. (And to all the other setters on nsMailtoUrl as well, of course.)
So rather than - NS_IMETHODIMP nsMailtoUrl::SetPath(const nsACString &aPath) { return m_baseURL->SetPath(aPath); } we would have NS_IMETHODIMP nsMailtoUrl::SetPath(const nsACString &aPath) { m_baseURL->SetPath(aPath); return ParseUrl() } I'll create a patch in a bit for this (if I understand you correctly), and open a new bug for all the other code in nsSmtpUrl.cpp that should be changed.
That's what we'd have, yes. Please just fix all of them -- no need to split this small change into two separate bugs.
Taking.
Assignee: jkeiser → dgk
Attached patch Patch to add ParseUrl (obsolete) — Splinter Review
My compiler is dead at the moment. (I upgraded to VSCC v7 and it isn't behaving fully).
Attachment #113228 - Flags: review?(bzbarsky)
Comment on attachment 113228 [details] [diff] [review] Patch to add ParseUrl Why did you change all those functions that are not setters? (getters, Equals, etc, etc). Don't do that. Also, the indentation is screwy. Please follow the file's indentation.
Attachment #113228 - Flags: review?(bzbarsky) → review-
Attached patch Better patchSplinter Review
I got into too much of a rhythm and stopped looking at the names. Here is a better patch.
Attachment #113228 - Attachment is obsolete: true
There will be a slight delay as I compile my tree and test the patch.
Comment on attachment 113232 [details] [diff] [review] Better patch My compiler is getting better, but currently can't build Mozilla (is breaking on XPIDL).
Attachment #113232 - Flags: review?(bzbarsky)
Shouldn't the return value of SetPath (and friends) also be checked, not only the one of ParseUrl?
Comment on attachment 113232 [details] [diff] [review] Better patch sr=bzbarsky; this needs review from a mailnews person. Seth, would you mind doing the honors?
Attachment #113232 - Flags: superreview+
Attachment #113232 - Flags: review?(sspitzer)
Attachment #113232 - Flags: review?(bzbarsky)
biesi: in theory, yes. In practice, I'm going with parity with SetSpec for now. ;)
re Comment #16, I think that any return value should be checked (especially in a c program), however, the symptoms explained in this bug has to be fixed as it is a 1.3b blocker. Mind you, if you want to file a seperate bug on the issues you brought up, go for it as they are valid issues. bz: Thanks for the sr, I wasn't planning on seeking that until I'd compiled and checked the fix. (Yup, I finally got MSVC v7 working....well compiling code anyway).
Comment on attachment 113232 [details] [diff] [review] Better patch r=sspitzer, assume you've tested this and regular old mailto urls work fine. (I can't see why they wouldn't, since we do this in SetSpec() already)
Attachment #113232 - Flags: review?(sspitzer) → review+
Comment on attachment 113232 [details] [diff] [review] Better patch Could this please be approved for 1.3b? David, do you need me to check this in?
Attachment #113232 - Flags: approval1.3b?
bz: don't check it in just yet. I want to double check that it isn't going to break something bad (not that it should).
Well, it still needs approval too. ;) Lemme know when.
Testing complete. On my local build, I duped the problem. Then, with the patch applied, the Mail/Compose window came up with all the values entered, which is the correct action. However, I would have thought that it would auto-send, but that is a different issue. bz: Once this gets an "a", can you check it in for me please?
Status: NEW → ASSIGNED
> However, I would have thought that it would auto-send That would 1) violate the RFC and 2) be a major privacy issue. ;) I can check this in, sure.
Comment on attachment 113232 [details] [diff] [review] Better patch a=asa (on behalf of drivers) for checkin to 1.3beta.
Attachment #113232 - Flags: approval1.3b? → approval1.3b+
fixed
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
> I would have thought that it would auto-send bz is right, that would be very, very bad.
Re: autosend I haven't read the RFC that covers form submissions via email, but I have seen what other browsers do (Netscape 4.x and IE 5.x). When you click on the Post/Submit button, the eMail goes to its destination without any user interaction. However, I do like the Mozilla method, it just seems more secure if you know exactly what you are sending in response to the form.
verifying build 2003-02-12-08
Status: RESOLVED → VERIFIED
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: