-1

I have checked all the similar questions to this but not found anything that helped... so here goes!

I am writing designing a site for my college project. It simply is an image gallery. I have a counter displayed for each image that increments each time the image is clicked. When the page refreshes the new number is displayed. With me so far?

The problem is that after the database update is completed the return does not complete the rest of the code...

echo "<div class='gridImg'><a href=".$imgpath." data-lightbox='countryside' data-title='".$row['ldesc']."' onclick='"."showUser(&#39".$fname."&#39)'>";

The above line is in a php file and the function in question is showUser, which passes a variable $fname...

function showUser(str) {
    if (str === "") {
        document.getElementById("txtHint").innerHTML = "";
        return;
    } else { 
        if (window.XMLHttpRequest) {
            xmlhttp = new XMLHttpRequest();
        } 
        xmlhttp.onreadystatechange = function() {
            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("txtHint").innerHTML = 
                this.responseText;
            }
        };
        xmlhttp.open("GET","../php/countrysideupdateviews.php?q="+str,true);
        xmlhttp.send();
        return;
    }
}

The above script takes the passed value and hands it over to countrysideupdateviews.php (Sorry if this script is a mess, I am new to AJAX and took it from the W3Schools site.

<?php
$q = $_GET['q'];
$conn=new mysqli('localhost','user','pass','dbname');
$sql="UPDATE countryside SET views = views + 1 WHERE fname = '".$q."'";
$result=$conn->query($sql);
mysqli_close($conn);
?>

The above php file updates the database.

Ok...

So, a user clicks on one of the images on-screen, which opens a lightbox gallery, BUT also updates the view count and then returns control - except that everything works - the update takes place - but the lightbox does not start, instead a static larger image of the one that was clicked is shown. The only way to clear it is to refresh the site, which does reflect the updated counter.

I have added returns to the onclick function call which does return control but the counter is not updated. Where am I going wrong? Bear in mind please I am still learning and I hope this makes sense :)

RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
cardshark
  • 21
  • 4
  • 1
    Your code is vulnerable to [SQL injection attacks](https://en.wikipedia.org/wiki/SQL_injection). You should use prepared statements with bound parameters, via either the [mysqli](https://secure.php.net/manual/en/mysqli.prepare.php) or [PDO](https://secure.php.net/manual/en/pdo.prepared-statements.php) driver. [This post](https://stackoverflow.com/q/60174/6634591) has some good examples. – Luca Kiebel Apr 29 '18 at 16:10
  • Thanks for the edit RiggsFolly. Luca, I appreciate what you are saying and I realise that - but for the time being I just want the code to work. It is not going to be used in a live environment, just on a test server. Remember it is just for my college work and nothing else. – cardshark Apr 29 '18 at 17:09

2 Answers2

0

The argument to onclick must be a function. showUser("foo") is not a function. You're also missing event.preventDefault() which prevents the click action from opening the link.

Change your showUser to 

function showUser(str) {
  return function(event) {
    if (str === "") {
      document.getElementById("txtHint").innerHTML = "";
    } else {
      if (window.XMLHttpRequest) {
        var xmlhttp = new XMLHttpRequest();
      }
      xmlhttp.onreadystatechange = function() {
        if (this.readyState == 4 && this.status == 200) {
          document.getElementById("txtHint").innerHTML = this.responseText;
        }
      };
      xmlhttp.open("GET","../php/countrysideupdateviews.php?q="+str,true);
      xmlhttp.send();
    }
    event.preventDefault();
  };
}
apaatsio
  • 3,073
  • 1
  • 22
  • 18
  • I have made the changes @apaatsio, and although the lightbox code now works, the database is not updated :( – cardshark Apr 29 '18 at 17:19
  • I thought the error would be in the php file that updates the database, as I know it works. Confused. – cardshark Apr 29 '18 at 17:20
0

Ok, after playing around with @apaatsio's answer I got it working, this is what the function now looks like...

function showUser(str) {
  if (str === "") {
    document.getElementById("txtHint").innerHTML = "";
    return;
  } else { 
  if (window.XMLHttpRequest) {
  // code for IE7+, Firefox, Chrome, Opera, Safari
    xmlhttp = new XMLHttpRequest();
    } 
    xmlhttp.onreadystatechange = function() {
    if (this.readyState == 4 && this.status == 200) {
      document.getElementById("txtHint").innerHTML = this.responseText;
      }
    };
  xmlhttp.open("GET","../php/countrysideupdateviews.php?q="+str,true);
  xmlhttp.send();
  event.preventDefault();
  }
}

It looks like preventing the click opening the link worked just fine - thanks :)

cardshark
  • 21
  • 4