1
\$\begingroup\$

Here is a script that opens about 200 pictures, the first time you open the webpage, loading of the 200 pictures is fast, However if you reopen the page, it takes about 4 minutes to load the same page the second time. Please advise.

    session_start();
    header('Expires: Mon, 26 Jul 1997 05:00:00 GMT');
    header('Cache-Control: no-store, no-cache, must-revalidate');
    header('Cache-Control: post-check=0, pre-check=0', FALSE);
    header('Pragma: no-cache');


    if(!isset($_SESSION['token']) && (!isset($_SESSION['user']))){
        echo "<script language='javascript'>self.close();</script>";
    }
    require 'db.php';
    $upload_id = $_GET['id'];
    $client = $_GET['NAME'];
    ?>
    <!DOCTYPE html>
    <html>
    <head>
    <title>Display | <?=$client;?></title>

    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link rel="stylesheet" href="css/w4.css">
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
    <script src="https://ajax.aspnetcdn.com/ajax/jQuery/jquery-3.2.1.min.js"></script>
    <script src="https://code.jquery.com/jquery-1.12.4.js"></script>
    <script src="https://code.jquery.com/ui/1.12.1/jquery-ui.js"></script>
    <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js"></script>
    </head>
    <body>
    <?
    $myid = $_GET['myd'];
    $mydept = $_GET['dept'];
    $myrole = $_GET['role'];
    ?>

    <?
    //}
    // Retrieving Book mark page for this job (upload_id)
    $book = ("SELECT * FROM bookmark WHERE user_id = '".$_SESSION['id']."' AND upload_id='".$upload_id."'  ");
    $bk = mysqli_query($conn, $book) or die ("ERROR bookmark ".mysqli_error($conn));
    $bookmark = mysqli_fetch_assoc($bk);
    $page  = $bookmark['page_book'];


    ?>
    <div class="w3-container">
     <center>
    </div>
    <div class="w3-content w3-display-container">
    <?
    $s = ("SELECT count(USER_ID) as counter FROM upload_data WHERE UPLOAD_ID='".$upload_id."'");
    $r = mysqli_query($conn, $s) or die ("ERROR ".mysqli_error($conn));
    $c = mysqli_fetch_assoc($r);
    $all = $c['counter'];


    $sql = ("SELECT YEAR, FOLDER, USER_ID, FILE_NAME FROM upload_data WHERE UPLOAD_ID='".$upload_id."' ORDER BY USER_ID");
    $result = mysqli_query($conn, $sql) or die ("ERROR ".mysqli_error($conn));
    while($row = mysqli_fetch_assoc($result)){
        $file = $row['FILE_NAME'];
        //$all = $row['pages'];
        $year = $row['YEAR'];
        $folder = $row['FOLDER'];
        $id = $row['USER_ID'];
        $fileshow = "/unix/upload/$year/$folder/$file";
    ?>

    <div class="w3-display-container myPictures "  style="left: -15%; ">
        <img  src=<? echo $fileshow;?> style="width:130%; height: auto;" data-zoom-image=<? echo $fileshow;?>>
      <!--<div class="w3-display-bottomleft w3-large w3-container w3-padding-16 w3-black">
        <?echo $id; ?>
      </div>-->
    </div>
    <?
    }
    ?>

     <button class="button5"  style="position: fixed; width: 100%; left: 20px; top: 140px; height: 40px; width: 100px; border-radius: 2px; background-color: Transparent;" onclick="bookSave(0)">Save</button>&nbsp;
     <div id="display"  style="position: fixed; width: 100%; left: 30px; top: 372px;" ></div>&nbsp;
     <button  class="button2" style="position: fixed; width: 100%; left: 20px; top: 505px; height: 40px; width: 100px; border-radius: 2px; background-color: Transparent;;" onclick="plusDivs(-10)">&#10094;&#10094;10-</button>&nbsp;
     <font><div id="bookmark" style="position: fixed; width: 100%; left: 20px; top: 190px; height: 60px; width: 100px;"></div></font></center>

    <div id="w20" class="clock"  style="position: fixed; top: 20px; left: 60%;"></div>

    <div class="message"  style="position: fixed; top: 20px; left: 50%;"></div>
    <button class="button" style="position: fixed; width: 100%; left: 20px; top: 410px; height: 50px; width: 100px; border-radius: 2px;"  onclick="plusDivs(-1)">&#10094; 前へ</button>
    <button class="button" style="position: fixed; width: 100%; left: 20px; top: 300px; height: 50px; width: 100px; border-radius: 2px;"  onclick="plusDivs(1)">次へ &#10095;</button>
    <button  class="button2" style="position: fixed; width: 100%; left: 20px; top: 215px; height: 40px; width: 100px; border-radius: 2px; background-color: Transparent;" onclick="plusDivs(10)">+10&#10095;&#10095;</button>&nbsp;
    </div>
    <?$hour = $_GET['t'];?>
    <!---------------------------------------- Next | Prev Functions ------------------------------------------------------->
    <script>
    var slideIndex = 1;
    showDivs(slideIndex);

    function plusDivs(n) {
      showDivs(slideIndex += n);
    }

    function showDivs(n) {
      var i;
      var x = document.getElementsByClassName("myPictures");
      if (n > x.length) {slideIndex = 1}
      if (n < 1) {slideIndex = x.length}
      for (i = 0; i < x.length; i++) {
         x[i].style.display = "none";
      }
      x[slideIndex-1].style.display = "block";

      var xhttp = new XMLHttpRequest();
      xhttp.onreadystatechange = function() {
        if (this.readyState == 4 && this.status == 200) {
          document.getElementById("display").innerHTML = this.responseText;
        }
      };
      xhttp.open("POST", "timer5.php", true);
      xhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
      xhttp.send("index="+slideIndex+"&name=<?echo $client;?>&upload_id=<?echo $upload_id;?>&total=<?=$all;?>&myid=<?=$myid;?>&dept=<?=$mydept;?>&role=<?=$myrole;?>");
    <?if($_SESSION['id']==20000){?>
    function clock1(){
    var clock;

            $(document).ready(function() {

                clock = $('.clock').FlipClock({
                    clockFace: 'MinuteCounter'
                });
            });
    }
    var clock;

            $(document).ready(function() {

                clock = $('.clock').FlipClock(<?=$hour;?>, {
                    clockFace: 'MinuteCounter',
                    countdown: true,
                    callbacks: {
                        stop: function() {
                            //$('.message').html('message here');
                            clock1();
                        }
                    }
                });

            });
    <?}?>
    }
    </script>


    <!-- ---------------------------------Save BookMark Functions ---------------------------------------->
    <script>
    function bookSave(n) {

    if(confirm('Save and Close?')){
     var xhttp = new XMLHttpRequest();
      xhttp.onreadystatechange = function() {
        if (this.readyState == 4 && this.status == 200) {
          document.getElementById("bookmark").innerHTML = this.responseText;
        }
      };
      xhttp.open("POST", "booksave.php", true);
      xhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
      xhttp.send("index="+slideIndex+"&name=<?echo $client;?>&upload_id=<?echo $upload_id;?>&myid=<?=$myid;?>");
     setTimeout("self.close()", 2000 ) // after 5 seconds
    }
     }
    </script>



    <!------------------------------------------------------ Call BookMark (PageNumber) when view ------------------------------------------------------------>
    <script>
    var slideIndex = <?=$page;?>;
    showBk(slideIndex);

    function bookMark(n) {
      showBk(slideIndex += n);
    }
    function showBk(n) {
      var i;
      var x = document.getElementsByClassName("myPictures");
      if (n > x.length) {slideIndex = 1}
      if (n < 1) {slideIndex = x.length}
      for (i = 0; i < x.length; i++) {
         x[i].style.display = "none";
      }
      x[slideIndex-1].style.display = "block";

      var xhttp = new XMLHttpRequest();
      xhttp.onreadystatechange = function() {
        if (this.readyState == 4 && this.status == 200) {
          document.getElementById("display").innerHTML = this.responseText;
        }
      };
      xhttp.open("POST", "callbookmark.php", true);
      xhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
      xhttp.send("index="+slideIndex+"&name=<?echo $client;?>&upload_id=<?echo $upload_id; ?>");
    }
    </script>
    </body>
    </html

>
\$\endgroup\$
2
\$\begingroup\$

I'm not sure why the script is slow... One might need to do some debugging to narrow down the timing to individual requests. Because this is the Code Review site, I will provide feedback and offer suggestions below.

SQL injection

The code is open to SQL injection, given that the SQL statements are executed with values coming straight from the super global $_GET without any filtering. While it is good that mysqli is used instead of the deprecated mysql library, PDO may be better as it may streamline the execution of statements. But nevertheless, it is recommended that you use bound parameters (e.g. with bind_param()) for the values instead of inserting them straight into the queries.

Multiple versions of jQuery added

I see 5 <script> tags and 4 of them appear to be different versions of jQuery

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
<script src="https://ajax.aspnetcdn.com/ajax/jQuery/jquery-3.2.1.min.js"></script>
<script src="https://code.jquery.com/jquery-1.12.4.js"></script>
<script src="https://code.jquery.com/ui/1.12.1/jquery-ui.js"></script>
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js"></script>

You should be able to remove the older versions to reduce loading time/size for users of your page.

jQuery DOM ready syntax

$(document).ready(function() { can be changed to $(function() { because the documentation for .ready() states:

jQuery offers several ways to attach a function that will run when the DOM is ready. All of the following syntaxes are equivalent:

  • $( handler )
  • $( document ).ready( handler )
  • $( "document" ).ready( handler )
  • $( "img" ).ready( handler )
  • $().ready( handler )

As of jQuery 3.0, only the first syntax is recommended; the other syntaxes still work but are deprecated.1

Repeated JS variable

I see the following line twice:

var clock;

The first one appears to be inside the function clock1, and the second one is outside that function. There isn't much need to re-declare that inside the function.

JS Selector for clock element

I see that $('.clock') is used to select the clock element, but there appears to only be one element with that class name:

 <div id="w20" class="clock"  style="position: fixed; top: 20px; left: 60%;"></div>

You could instead use the id selector for that element: $('#w20;') instead of the class selector...

code as string passed to setTimeout

I see this line at the end of the JavaScript function bookSave():

 setTimeout("self.close()", 2000 ) // after 5 seconds

According to the MDN documentation for setTimeout():

code
An alternative syntax that allows you to include a string instead of a function, which is compiled and executed when the timer expires. This syntax is not recommended for the same reasons that make using eval() a security risk.2

it would be better to use a function reference, like below:

setTimeout(self.close, 20000 ) //after 5 seconds

Notice that the function is not invoked (i.e. the parentheses are not added) when passed as a parameter here, lest it be called when the line is executed.

1http://api.jquery.com/ready/

2https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Parameters

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.