Fixing a mail form php file

Hey all,

So, I’m having trouble understanding what to fix to get this form to work. Can anyone look at it and tell me. The file : request-quote.html is the html for the form and the sendmail.php is the php file to mail the form…

here is the git:

The link you have given isn’t a mail form and in fact there is no code there.

ok try now, refresh page or click link again…please

That’s better. It is there now. Perhaps you can explain what it is that is not working for you.

ok

Well I thought I coded the sendmail.php file fine, but when I click send on the form (http://custom-animations.com/request-quote.html), the page goes to this page (http://custom-animations.com/sendmail.php) with an error (

)…

I get the following error:
Parse error: syntax error, unexpected ‘if’ (T_IF) in sendmail.php on line 3

I note that in your form you have id=“email” etc. I haven’t checked to see if you need the ids (I suspect not) but you certainly need to have name=“email” etc or the PHP script will not pick up the information from the form.
.

Ok I changed both files, but still getting the same error. Honestly, I don’t understand the error of line 3, could you explain tome why it’s wrong? please…

I did the changes in the master of the git repo so that you can see what they are…

is the sendmail.php correctly coded otherwise? Is everything where it’s supposed to be? Anything missing?

Line 62:

    $phone-number = $_POST['phone-number']; // required

I don’t think you can use a minus-sign in a PHP variable name, only an underscore. Same on line 68 and anywhere you use those two variables.

The errors I am getting seem to be caused by unprintable characters on almost every line, but I’m not sure that that would cause your 500 server error.

unprintable characters?

I’ve tried a few things…edits, I mean and it’s still not working…see the latest git commit.

I totally altered the sendmail.php script. Now, I’m not getting a 500 error but the mail isn’t being sent and upon clicking ‘Submit’ the page just goes blank…

See updated git… https://github.com/jannymac/custom-animations.git

That’s good news that you’re not getting an error now. My guess if you’re not getting anything is this line:

if (!$errName && !$errEmail && !$errHuman)

You’re performing a logical not operation on a string. I have no idea what that would evaluate as.

One other thing, you shouldn’t simply assign

$xxx = $_POST['xxx'];

without validating or sanitising the POST variable. Rather like you have done with $email later in your script.

Ok gandalf, thank you, so much. I’ll be real honest and say, under normal circumstances I’d have just got a form and sendmail script and changed the email_to and subject lines…but this was a form with custom fields, as requested by the client and not a standard contact us form, so I did the form from scratch using bootstrap, but have no idea what I’m doing in terms of the sendmail script - just following tutorials online or editing existing scripts.

I’m fighting to understand what you just said…can you edit it for me in git so that I see what you mean?

I’m still learning php and these things and this is sounding a bit much like gibrish, as my understanding isn’t fully matured yet…

If you are not yet confident enough with PHP to create a custom form processor, you could use a pre-made processor like this one:-

That will take a lot of the hard work out of it like anti-spam and security.

Looking at your html form (without getting into the php script), there is a problem with the checkboxes, the groups of checkboxes all ahve the same name.
The fix depends upon your intention.
Should users have a choice of just one box from the group? If so they should be radio type inputs.
If the user may choose any number from the group, each box should have a unique name or create an array, eg: name="vidtype[]"

So then I have to re-name then vidtype1, vidtype2, etc…? …because I can learn eventually how to do the array, but the client needs this right now, so the simpler methods for now, which is just to change the ‘name’, yea?

Do it which way you like, arrays may be better, but you may find unique names easier to understand.
Though looking at the options, maybe they should be radios, in which case they should keep the same names and change the type from checkbox to radio. That will mean users can only choose one from the group.

I changed it Sam…thanks.

I think your if statement should probably be:

if (!isset($errName) && !isset($errEmail) && !isset($errHuman))

but then you conditionally set $errName, $errEmail and $errHuman but don’t do anything with them (other than use them in the if statement).

PHP’s default mail() function sometimes doesn’t work and even if you have it working, it’ll most likely break within a week or so. I suggest using PHPMailer to avoid this. You are also using if(isset($_POST[...])) which is a very common amateur legacy hack. The proper way is using if($_SERVER['REQUEST_METHOD'] == 'POST'). I just spammed you with a “test” and I got a blank page. Seems like the page has errors. Do you have them logged? This is also a good reason why checking for proper error handling is a good idea and good practice. Because you might get a blank page such as this.

1 Like