Closed
Bug 191248
(mailto_broken)
Opened 22 years ago
Closed 22 years ago
mailto: Form submission broken
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
People
(Reporter: Matti, Assigned: d_king)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
2.40 KB,
patch
|
sspitzer
:
review+
bzbarsky
:
superreview+
asa
:
approval1.3b+
|
Details | Diff | Splinter Review |
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
![]() |
||
Comment 1•22 years ago
|
||
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
![]() |
||
Updated•22 years ago
|
Severity: normal → major
Keywords: regression
Comment 3•22 years ago
|
||
seems like a good one to get for beta. Who can own this?
Flags: blocking1.3b? → blocking1.3b+
Comment 4•22 years ago
|
||
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
Updated•22 years ago
|
Alias: mailto_broken
![]() |
||
Comment 5•22 years ago
|
||
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... ;) ).
Assignee | ||
Comment 6•22 years ago
|
||
bz: Are you saying that the fix for this would be to add ParseUrl() to
nsMailtoUrl::SetPath() ?
![]() |
||
Comment 7•22 years ago
|
||
Yes. (And to all the other setters on nsMailtoUrl as well, of course.)
Assignee | ||
Comment 8•22 years ago
|
||
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.
![]() |
||
Comment 9•22 years ago
|
||
That's what we'd have, yes. Please just fix all of them -- no need to split
this small change into two separate bugs.
Assignee | ||
Comment 11•22 years ago
|
||
My compiler is dead at the moment. (I upgraded to VSCC v7 and it isn't behaving
fully).
Assignee | ||
Updated•22 years ago
|
Attachment #113228 -
Flags: review?(bzbarsky)
![]() |
||
Comment 12•22 years ago
|
||
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-
Assignee | ||
Comment 13•22 years ago
|
||
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
Assignee | ||
Comment 14•22 years ago
|
||
There will be a slight delay as I compile my tree and test the patch.
Assignee | ||
Comment 15•22 years ago
|
||
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)
Comment 16•22 years ago
|
||
Shouldn't the return value of SetPath (and friends) also be checked, not only
the one of ParseUrl?
![]() |
||
Comment 17•22 years ago
|
||
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)
![]() |
||
Comment 18•22 years ago
|
||
biesi: in theory, yes. In practice, I'm going with parity with SetSpec for now. ;)
Assignee | ||
Comment 19•22 years ago
|
||
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 20•22 years ago
|
||
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 21•22 years ago
|
||
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?
Assignee | ||
Comment 22•22 years ago
|
||
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).
![]() |
||
Comment 23•22 years ago
|
||
Well, it still needs approval too. ;)
Lemme know when.
Assignee | ||
Comment 24•22 years ago
|
||
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
![]() |
||
Comment 25•22 years ago
|
||
> 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 26•22 years ago
|
||
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+
![]() |
||
Comment 27•22 years ago
|
||
fixed
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 28•22 years ago
|
||
> I would have thought that it would auto-send
bz is right, that would be very, very bad.
Assignee | ||
Comment 29•22 years ago
|
||
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.
Updated•6 years ago
|
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•