0

I have a php contact form that when submitted validates the fields but this process reloads the page and the form is not at the top of the page so I want it to automatically scroll to the bottom of the page where the form is when validation fails so the user can see the errors. It seems javascript is the only way to do this so I tried echo'ing a script along with the error message but it doesn't work. There are no errors in the browser console, just doesn't scroll.

I took the parsed HTML from the View Source after the new page loads and put it into jsFiddle here. You can see that it scrolls properly in the fiddle but the real site doesn't.

EDIT:
I also tried adding the loading of the jquery library to immediately before the scroll script and it still didn't scroll even though I confirmed the library is loading first. I'm at a loss.

This is a snippet of the php:

<?php
// define variables and set to empty values
$nameErr = $fromErr = $phoneErr = $verif_boxErr = $recaptchaErr = "";
$name    = $from = $phone = $message = $verif_box = "";
$recaptcha = NULL;
$errors  = 0;

if ($_SERVER["REQUEST_METHOD"] == "POST") { //check if form has been submitted
    if (empty($_POST["name"])) {
        $nameErr = " * Name is missing";
        $errors  = 1;
        echo '<style type="text/css"> input#name {border: 1px solid #F00; 
        box-shadow: 0px 0px 5pt .1pt #F00 inset;}</style>
        <script type="text/javascript">
function scrollSmoothToBottom(){ 
$(scrollingElement).animate({scrollTop:document.body.scrollHeight},500)
}scrollingElement=document.scrollingElement||document.body,
window.onload=scrollSmoothToBottom;</script>';
    } else {
        $name = test_input($_POST["name"]);
        // check if name only contains letters and whitespace
        if (!preg_match("/^[a-zA-Z ]*$/", $name)) {
            $nameErr = "Only letters and white space allowed";
            $errors  = 1;
            echo '<style type="text/css"> input#name {border: 1px solid #F00; box-shadow: 0px 0px 5pt .1pt #F00 inset;}</style>';
        }
    }

if ($errors == 0) { // all fields successfullty validated. 
        $message = "Message: " . "\n" . $message;
        $message = "Name: " . $name . "\n\n" . $message;
        $message = "Email: " . $from . "\n" . $message;
        mail("website@avayoupaint.com", 'Contact Form: ' . $name, $message = "Sender IP Address: " . $_SERVER['REMOTE_ADDR'] . "\n\n" . $message, "From: website-html@avayoupaint.com");            
        setcookie('tntcon', '');    // delete the cookie so it cannot sent again by refreshing this page
        header('Location: success.php');    // redirect to success page
        exit();
}


}
?>
<html>
<head></head>
<body>
<article id="contactForm">                        
    <span class="error">* All fields are required</span>
    <form action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]);?>" method="post" name="form1" id="form1">
    <span class="contactTitles">Name:</span>
    <input name="name" type="text" id="name" value="<?php echo $name;?>"/><span class="error"><?php echo $nameErr;?></span>
    </form> 
</article>
</body>
</html>
MilkyTech
  • 1,919
  • 2
  • 15
  • 39
  • I went to your [real site](https://www.avayoupaint.com/contact.php) (link from your deleted question). I copied the code into a blank file (rather than using JSFiddle, which can sometimes distort things). I added `console.log(scrollingElement);` to the `scrollSmoothToBottom` function. It runs, but logs `null`, so it isn't finding the element. This is because the script is loading and executing before the HTML document is rendered - the window might be loaded, but the document isn't. Therefore it can't find the body element to attach the "animate" function to. – ADyson May 12 '20 at 09:01
  • Two possible solutions: 1) move the script to the bottom of the page, just before the closing `

    ` tag, so it doesn't execute until all the HTML is loaded (browsers load and execute each tag the moment they find it, they don't wait for everything to load). 2) Wrap the script in a `$(document).ready` block (as I suggested way back in the previous chat already) so it doesn't execute until all the HTML is ready - because that "ready" event doesn't fire until then.

    – ADyson May 12 '20 at 09:04
  • Either way, you should also 1) ensure the script is within either the `` or `` tag of your page, otherwise technically it makes the HTML document invalid. Some browsers may tolerate that and run the script anyway but you shouldn't rely on it. 2) Make sure you only load jQuery once into your page - right now you're loading it twice in two different places - that can sometimes cause weird side-effects and problems. 3) Use a newer version of jQuery. The 1.x branch is only needed if you have to support really old browsers like IE8, but no-one should be needing that anymore really. – ADyson May 12 '20 at 09:06
  • P.S. 90% of the code you've posted in your question above is completely irrelevant to the problem. It's unclear why you would expect the server-side function which sends an email to have any bearing on a client-side Javascript problem relating to scrolling? Yet you didn't include any code which would actually point towards the problem. All you included was a link to a JSFiddle which _didn't_ reproduce the issue (probably because JSFiddle wraps all the code you post in another document object (necessarily, so it can put it in that little frame), so it found the element, and masked the issue. – ADyson May 12 '20 at 09:09
  • It's lucky I can view deleted questions and retrieved the link to the real site, otherwise we would still have no real clue about the actual problem. – ADyson May 12 '20 at 09:29
  • I didn't put the script at the very top of the document like it is in the jsFiddle. The server-side function does that. The script is part of a php variable for an error in the form. I don't want to scroll to the bottom when the contact page is loaded, only when the page automatically reloads on submit with an error in the form. The php posted above is the portion that contains the script and the portion that executes when submit is clicked. jquery is only loading twice right now because I was experimenting with when to load it so I added it to the error variable just before the scroll. – MilkyTech May 12 '20 at 12:12
  • I don't think I can change where I have the script in the original php for it to do what I want when I want. I will try the `$(document).ready` block you suggested. Perhaps an actual answer with how to implement that into my script in the code above would be the accepted answer I'm looking for. If you look at the posted code, you can see my "from" email address so you could have figured out the website without the deleted post. – MilkyTech May 12 '20 at 12:18
  • "I don't think I can change"..why not? Just put the value in a string instead of echoing it directly. Then later in the script, check if the string has anything in it, and if so, echo it there. – ADyson May 12 '20 at 12:24
  • P.S. as an aside, this bit: `$referer = $_SERVER['HTTP_REFERER']; // Get the referring URL $this_url = "https://" . $_SERVER['HTTP_HOST'] . $_SERVER["REQUEST_URI"]; // Get the URL of this page // If the referring URL and the URL of this page don't match then // display a message and don't send the email. if ($referer != $this_url) { echo "You do not have permission to use this script from another URL, nice hacking attempt."; exit;` is unreliable because REFERER is not always supplied, and can also be spoofed easily – ADyson May 12 '20 at 12:27
  • If you want to stop people posting to your form from outside the domain, you'd normally use a [CSRF token](https://www.netsparker.com/blog/web-security/protecting-website-using-anti-csrf-token/), or...you'd use a captcha, which you've already done. So that should already prevent other sites or bots from posting to the form, because they won't be able to complete the captcha directly. So I think you can just remove all that REFERER-related code completely, it's redundant. – ADyson May 12 '20 at 12:29
  • I an not nor have ever claimed to be some kind of php expert. I barely understand it enough to modify a friends code to work on my website. I took the code from a website we worked on together many years ago but he wrote all of the php. That's not my bag. I will delete the hack protection. I did try the `$(document).ready` block but I get an error in the console: "Uncaught RefenceError: $ is not defined". I will try putting the script in the html where I `echo $nameErr` – MilkyTech May 12 '20 at 12:42
  • _"I an not nor have ever claimed to be some kind of php expert"_. I know. That's fine. We all start off the same way. I've been using PHP on and off for 15 years and there's still tons of stuff I don't know or haven't used yet. But I do know a reasonable amount (both about PHP and about the web in general), and that's why I'm giving you advice on it. It's not a criticism, I'm just laying out the facts and showing how you could improve the code. Stop taking it personally! :-) – ADyson May 12 '20 at 12:47
  • "$ is not defined" means you didn't include jQuery in the page - or at least not before that ` – ADyson May 12 '20 at 12:48
  • You can prevent the page from refreshing https://stackoverflow.com/questions/45735757/stop-page-refresh-after-form-submit – Yotam Dahan May 12 '20 at 14:17
  • @YotamDahan I'm going to try that once I get this functioning perfectly this way. I'm almost there. I previously looked into validating without submitting but all the solutions were validating with javascript which would require completely recoding the entire contact page to validate with js instead of php. That being said, that post looks promising. I don't completely understand it yet but I will get back to it after I finish this. – MilkyTech May 12 '20 at 14:58

1 Answers1

0

Your script to get the scrollingElement variable runs immediately when the page loads, before all the HTML is ready and loaded into the page. You need to wait until all the content is loaded, otherwise the script will not find the document.body object to attach the scroll animation to. This is especially a problem because the script is positioned before the HTML document. Browsers execute script as soon as they receive it, they don't wait until the whole document is loaded.

jQuery provides the document.ready event handler for you to wrap your Javascript in, so that the main script waits for the page to be in the state where all the HTML has loaded (there's also a native JS way to do the same thing, but since you're using jQuery we'll do it this way). You actually already have an example of this in your current site, right down near the bottom.

It also makes more sense for the scrollingElement to be inside the function - it doesn't have much purpose being a global, as it's only used within that one function.

<script type="text/javascript">
  $(document).ready(function() {

  function scrollSmoothToBottom() { 
    var scrollingElement = document.scrollingElement || document.body;
    $(scrollingElement).animate({ scrollTop:document.body.scrollHeight }, 500);
  }
    scrollSmoothToBottom();
  });
</script>

Note that this version requires jQuery to be loaded before this script block runs. It would make sense to output the script block somewhere in the <head> section of the page, just below a <script block which loads jQuery.

ADyson
  • 57,178
  • 14
  • 51
  • 63
  • I thought the `window.onLoad` meant that the browser DID wait for the entire document to load. I will try this if I am unsuccessful moving the script into the html. – MilkyTech May 12 '20 at 12:46
  • Yes it does. It's similar to document.ready/DOMContentLoaded. So you could stick with onload I guess for that bit if you want. But if you look at your original code carefully, the bit where you define `scrollingElement` was not part of the onload event, that line was executed _before_ the onload - it would run immediately when the script block was initialised, hence why the value of `scrollingElement` was null when you came to use it in the function. The only bit triggered by onload in your code was the call to the function. – ADyson May 12 '20 at 12:52
  • this didn't work. I get the same "Uncaught RefenceError: $ is not defined" as when I tried it. I was actually using `$(document).ready` on my first attempts before switching to `window.onLoad` due to the error. – MilkyTech May 12 '20 at 12:54
  • So you _could_ probably solve it by simple moving `var scrollingElement = document.scrollingElement || document.body;` inside the function as I've done anyway, and keeping the rest of your code the same. But you'll still need to ensure jQuery is loaded before this script runs, because of the `$(scrollingElement).animate` command. If you've kept your reference to jQuery lower down the page, and you stick with onload (which waits for all script files etc to load as well as HTML, then you can probably get away with it, but the document.ready event would happen earlier as it only waits for HTML) – ADyson May 12 '20 at 12:54
  • btw - I just added the recaptcha yesterday so the hacking code wasn't redundant prior. – MilkyTech May 12 '20 at 12:54
  • ok well that's a good step you've taken in doing that, because the hacking code, while not completely redundant, would probably fail to detect a problem 50% or more of the time, and/or could be defeated by a very simple bit of code on the client side. But it's good to remove code that's not needed, because then you don't have to maintain it, it declutters the rest of the code, and you don't get confused when you come back to the code months later and start to wonder whether that bit is necessary or not (and then have to test it all over again to be sure). – ADyson May 12 '20 at 12:55
  • I just tried putting the script down in the html where I echo the error variable and the page scrolls but it also scrolls when you first load the contact page. I only want the scroll to occur after the form is submitted with errors. – MilkyTech May 12 '20 at 13:07
  • I tried it with your code exacty. `var scrollingElement...` Inside the function. Still get the `$ is not defined` error. – MilkyTech May 12 '20 at 13:11
  • that's because of the $.animate function...jQuery still needs to be loaded first before the script runs. Easiest way to do that is to ensure that the jQuery script block is above this one in the page – ADyson May 12 '20 at 13:14
  • If it scrolls when the form is first loaded (not just after an error) I guess that means you echo'd the script without checking if the form had an error. You had that part sorted before, so without seeing exactly what you did, I can't tell why that behaviour has regressed. – ADyson May 12 '20 at 13:15
  • You'd need to check `if ($errors == 1)` to decide whether to echo the script – ADyson May 12 '20 at 13:21
  • ok...jquery loading first gets rid of the `$ is not defined` error but then I found a problem with your code. The function you call is not the same name as the function you define. Fixed that and now it works! but it doesn't scroll smooth. It appears to instantly jump to the bottom with `500` so I switched it to `1000` and its better but still not smooth because it seems like it is trying to scroll at the same time that other elements on the page are still loading. – MilkyTech May 12 '20 at 13:29
  • Now I have another issue as well. The only way I can get jquery to load before the script is to include the jquery script inside the error variable but now if I remove jquery from the bottom of my `body` tag, I have no jquery when the regular contact page is loaded. I only have it when there is a form error. – MilkyTech May 12 '20 at 13:34
  • oh sorry the function name thing is a typo on my part. I'll correct it now – ADyson May 12 '20 at 13:48
  • maybe go back to using onload then, to try and correct the timing. – ADyson May 12 '20 at 13:50
  • "The only way I can get jquery to load before the script is to include the jquery script inside the error variable"....I've already addressed this point. load the jQuery script inside the head of the page, and then echo the scrolling script just after it (but only if $errors == 1) – ADyson May 12 '20 at 13:50
  • so can I put all of the php inside the head? Right now all of the php is before all of the html. The code above is basically all of the php. The only difference is a few more validation checks for the other form fields. – MilkyTech May 12 '20 at 14:20
  • I put the php inside the head and it works fine so now I can probably try going back to `window.onLoad`. Although not a php guy, I always thought is was correct practice to have the php before the html. Does it really matter? – MilkyTech May 12 '20 at 14:34
  • 1
    You can mix it up a bit. You don't need _all_ your PHP inside the head. You'd only need this much: ` $(document).ready(function() { function scrollSmoothToBottom() { var scrollingElement = document.scrollingElement || document.body; $(scrollingElement).animate({ scrollTop:document.body.scrollHeight }, 500); } scrollSmoothToBottom(); }); '; } ?>` . The rest of it can be left where it is, so that the $errors variable is ready to be checked by the time the head is being generated. – ADyson May 12 '20 at 14:41
  • 1
    But in general, you should separate PHP which deals with business logic, validation, talking to databases etc into the top of the script (and/or into separate classes and functions which are in totally separate script files, and only get `include`d in your main script, so they're maintainable separately and re-usable), but if you need some PHP to control the final output sent to the browser, then it's acceptable and clearly logical to include that in the appropriate place within the general static HTML. – ADyson May 12 '20 at 14:44
  • I now have it with the `if` statement in your comment. works great but still not smooth to went back to `window.onLoad` but would only work if I moved the script inside the `body` tags. Is that ok to be in the body instead of the head? – MilkyTech May 12 '20 at 15:28
  • I tried leaving it in the head and changing `document.body` to `document.head` but that didn't work. – MilkyTech May 12 '20 at 15:29
  • yeah it can be inside the body, that's not a crime at all. – ADyson May 12 '20 at 15:34