PHP e-mail contact form with AJAX

Question

I have just started learning how to build websites, and I have been experimenting by creating a contact form.

Here is what I currently have - can anyone suggest and recommend ways enhance my current contact form system?

JavaScript:

$("#contactForm").submit(function (event) {

    /* stop form from submitting normally */
    event.preventDefault();

    /* get some values from elements on the page: */
    var $form = $(this),
        $submit = $form.find('button[type="submit"]'),
        name_value = $form.find('input[id="name"]').val(),
        email_value = $form.find('input[id="email"]').val(),
        message_value = $form.find('textarea[id="message"]').val(),
        url = $form.attr('action');

    /* send the data using post */
    var posting = $.post(url, {
        name: name_value,
        email: email_value,
        message: message_value,
        ajax: 1
    });

    posting.done(function (data) {
        $form.find('span.error').remove();

        if (data == "1") {

            /*Change the button text.*/
            $submit.text('Sent. Thank You!');
            $submit.addClass("sent");

        } else {
            $submit.after('<span style="display: inline-block; padding: 20px 5px; color: #bd3d3d" class="error">Failed to send the message, please check your entries and try again.</span>');
            /*Change the button text.*/
            $submit.text('Try Again');
        }
    });
});

PHP

<?php

    if(isset($_POST['name']) && $_POST['email'] && $_POST['message']) {

    $name = $_POST['name'];
    $email = $_POST['email'];
    $message = $_POST['message'];

    $to = "[email protected]";
    $subject = "New Message From: $name";
    $message .= "$messege";

    $headers  = 'MIME-Version: 1.0' . "\r\n";
    $headers .= 'Content-type: text/html; charset=utf-8' . "\r\n";
    $headers .= 'From: '.$email . "\r\n";
    $mailed = ( mail($to, $subject, $message, $headers) );

    if( isset($_POST['ajax']))
        $response = ($mailed) ? "1" : "0";
    else
        $response = ($mailed) ? "<h2>Success!</h2>" : "<h2>Error! There was a problem with sending.</h2>";
        echo $response;
        }
    else
        {
        echo "Form data error!";
    }
?>

HTML:

<form id="contactForm" action="email.php" method="post" name="contactForm">
    <div class="name">
        <label>Your Name</label>
        <input id="name" type="text" placeholder="Enter Name" required>
    </div>
    <div class="email">
        <label>Email Address</label>
        <input id="email" type="email" placeholder="Enter Email" required>
    </div>
    <div class="message">
        <label>Message</label>
        <textarea id="message" required></textarea>
    </div>
    <button id="submit" type="submit">Send</button>
</form>

Answer 1

Php

I'm not a big Php guy so I can't be too thorough in this section. The biggest thing is that you seem to be writing ... well ... a php script. While that absolutely works it's incredibly old-school and doesn't do anything as far as guiding you into the pit of success. So first bit of advice: use a framework. At time of writing, the two I've been hearing most buzz about are Laravel and Codeigniter. This is constantly changing but just pick one which you've heard about and use it, that will help you learn enough concepts to make an educated choice next time you need to select one.

Second, at the bottom you seem to have some invalid php. You have some missing braces in that if statement toward the bottom and it has two else clauses in a row. I'm not sure if that will cause an error but it will definitely never run the third clause.

Finally, your design leaves open a huge security hole if the site is ever opened to the public. You accept the message as an input parameter, much worse, you accept the email. Think of what happens when someone does a query to yoursite.com/yourpage.php?email=yourgrandma&message=Send%20me%20all%20the%20moneyand again, and again. Spammers love finding unsecured services like this!

A much better approach is to have the emails (and maybe message) stored on the server already in the database or a text file. Then you could supply parameters like email=1 indicating "email the person with id=1".

Having this behind a login page is not a (good) defense either. Someone could easily put the above url in the src of an <img> element and every time someone who had recently logged into your site visits a page they put that on (like a comment or forum page) you will be sending spam! To defend against this make sure that page does not accept the Http GET verb (in this situation POST would be appropriate). This is exactly the kind of error that a framework shoudl help defend you from.

Finally, I see what you're doing with the ajax flag but that's not the typical thing to do. Whether the operation succeeded or not should be determined by the returned Http code (200 for success and maybe 400 for error). Whether you should be returning json or html should be determined by a concept called content negotiation. This is again, something that your framework should be handling for you.

Html

name is not a valid attribute for the <form> element. But also don't put ids on things unless you absolutely know there will be only one of these elements on the page. The spec says ids must be unique across the page and lots of javascript libraries cause errors when the assumption is broken. In this case can you guarantee there will not be two contactForm elements on the page at once? What if you have 2 offices and each one wants its own contact form? What about elements with id=message or id=submit? A better approach I think in this case is to use a class. That is not to say that you should never use ids but in most cases the benefit isn't worth the risk.

Next, you don't technically need <button type=submit>, it's optional but submit is the default behavior.

Finally, I'm not 100% sure of the spec for <label> but I don't think it works by associating the label with the following <input>. Instead you have to use the for attribute and ids (which I've already argued is bad). Or do what I recommend, something like:

    <label class="email">
        <span class="label">Email Address</span>
        <input id="email" type="email" placeholder="Enter Email" required>
    </label>

Javascript

Javascript naming convention are to use camelCase with no underscores. It's not a big deal but it's pretty standard. The only exception is to use PascalCase for variables that are intended to be used with the new keyword (which you don't have here).

To a lesser extent than php, but with the javascript community where it is, its probably time to start considering choosing a javascript framework (like angular, backbone, knockout, or the like rather than using jquery directly.

In my opinion you are over-specifying your selectors, no need to do input[id=foo] when id is already specified as unique, no need to span.blah when span holds no semantic nor structural meaning. Consider that all this stuff is indexed in the browser so by adding more selectors you might actually be slowing things down however slightly.

You don't need to use $('id=something') to select, you can just say $('#something')

Your scope management looks pretty good, I'm happy to see you using find - another syntax which achieves the same purpose and you might find flows better (or not) is $('[name=email]', $form).

You've got inline styling and html. Why? Just put the element on your html page and show or hide it from javascript.

Finally, just about everything you're doing in js is handled in a very elegant manner by the jquery.form plugin. Just use that.

Answer 2

1. Use a framework. As already mentioned

   • Using a framework provides so much for free. It provides everything you've not provided in your code above. For example: error/exception handling, markup generation, logging, filters, validators, security...the list is long.

2. Completely remove the javascript.

   • There is no benefit from using ajax to submit the form. On the other hand, you've just incurred useless debt in your application by including the javascript. Let the browser simply submit the form. Don't use ajax for ajax's sake.

3. If you were to deploy your code as-is be sure you have a great spam filter for your email-to address, Or use some sort of "are-you-human" detector.

   • I actually don't know a great way to protect contact forms...it seems the spammers keep defeating almost all efforts.

Answer 3

Edit: your PHP code is a little messy, I almost couldn't tell what was going on in the if statements

<?php

  if(isset($_POST['name']) && $_POST['email'] && $_POST['message']) {

  $name = $_POST['name'];
  $email = $_POST['email'];
  $message = $_POST['message'];

  $to = "[email protected]";
  $subject = "New Message From: $name";
  $message .= "$messege";

  $headers  = 'MIME-Version: 1.0' . "\r\n";
  $headers .= 'Content-type: text/html; charset=utf-8' . "\r\n";
  $headers .= 'From: '.$email . "\r\n";
  $mailed = ( mail($to, $subject, $message, $headers) );

  if( isset($_POST['ajax']))
      $response = ($mailed) ? "1" : "0";
  else
      $response = ($mailed) ? "<h2>Success!</h2>" : "<h2>Error! There was a problem with sending.</h2>";
      echo $response;
      }
  else
      {
      echo "Form data error!";
  }
?>

so I would fix the indentation of your code so that it was readable, first, which would give us this

<?php

    if(isset($_POST['name']) && $_POST['email'] && $_POST['message']) {

        $name = $_POST['name'];
        $email = $_POST['email'];
        $message = $_POST['message'];

        $to = "[email protected]";
        $subject = "New Message From: $name";
        $message .= "$messege";

        $headers  = 'MIME-Version: 1.0' . "\r\n";
        $headers .= 'Content-type: text/html; charset=utf-8' . "\r\n";
        $headers .= 'From: '.$email . "\r\n";
        $mailed = ( mail($to, $subject, $message, $headers) );

        if( isset($_POST['ajax']))
            $response = ($mailed) ? "1" : "0";
        else
            $response = ($mailed) ? "<h2>Success!</h2>" : "<h2>Error! There was a problem with sending.</h2>";
        echo $response;
    }
    else {
        echo "Form data error!";
    }
?>

Then I notice that you don't have any braces surrounding the nested if/else statement and almost thought that the echo was supposed to be inside of that else statement, but I knew better. I would add braces like this

if( isset($_POST['ajax'])) {
    $response = ($mailed) ? "1" : "0";
} else {
    $response = ($mailed) ? "<h2>Success!</h2>" : "<h2>Error! There was a problem with sending.</h2>";
}
echo $response;

Then we can move the echo into the if/else statement like this

if( isset($_POST['ajax'])) {
    echo ($mailed) ? "1" : "0";
} else {
    echo ($mailed) ? "<h2>Success!</h2>" : "<h2>Error! There was a problem with sending.</h2>";
}

I like returning from inside the if statements if I can.

---

To obscure your E-mail from a spam bot but not from a user I use Google Captcha's, they are really cool. and you can either put a captcha on your input form, or you can put a captcha on your E-mail address

Google Recaptcha is the place to go. if Google can't do it right, who can?

it's a simple plugin, took me an hour or two, but I have ADHD and am Distracted very easily, so I don't see any reason this should take more than 30 minutes to an hour to implement into your page.

Note:

I just answered based off of another answer assuming what the question entailed.