CODEWITHSUNDEEP
javascript
HandsomeRob
HandsomeRob

Reputation: 445

DOM1 Javascript not working

I probably did this the wrong way, I learned Jquery first and easy stuff. Taking a course in javascript learning DOM0 & DOM1 and just cannot get this to work.

<!DOCTYPE html>

<html lang="en">

<head>
<script type="text/javascript">
     function addBorder(){
          for var i=0; i<document.getElementsByTagName('img').length; i++) {
               document.getElementsByTagName('img')[i].style.border="3px solid red");
          }
     }
</script>
<script type="text/javascript">
     window.onload=addBorder;
</script>
</head>

<body>
<img src="image1.jpg" />
<img src="image2.jpg" />

</body>

</html>

Quickly typed, ignore typos if there are any. Results in no errors and no activity.

Tried it all different ways, DOM0 document.images..., window.onload=function(){....

Always nothing. Get the basic idea that images are loading after script, if placed in body it works fine. But why would it not work within window.onload=??

Upvotes: 1

Views: 144

Answers (2)

msanford
msanford

Reputation: 12227

You have a typo in your source code, as Ryan pointed out (you need a space between var and i).

Even though this is a beginner class you might want to get into good habits right from the start. Generally, looking at the DOM should be done as rarely as possible. Your function goes to the DOM every time you invoke document.getElementsByTagName('img'). Since it's in a for loop, that might be a lot. Granted, in your example there are only two img tags, but think about applying this script to a real page.

It's usually a good idea to touch the DOM once and cache the result in a variable, like so:

function addBorder() {
      var images = document.getElementsByTagName('img');
      for (var i=0; i < images.length; i++) {
           images[i].style.border = "3px solid red";
      }
}

This way you only hit the DOM once, then use the variable images (which is scoped to your function).

Also, you can (should) put those both in the same script tag in your HTML document (since they're inline scripts). And since you're only calling one function, and you're calling it on window.onload, you can simplify it and wrap it in an immediately invoked function expression (further reading), like so:

<!DOCTYPE html>
<html lang="en">
<head>
<script type="text/javascript">

    (function () {
          var images = document.getElementsByTagName('img');
          for (var i=0; i < images.length; i++) {
               images[i].style.border = "3px solid red";
          }
    })();

</script>
</head>

<body>
  <img src="image1.jpg" />
  <img src="image2.jpg" />
</body>

</html>

Upvotes: 1

Ryan O&#39;Neill
Ryan O&#39;Neill

Reputation: 3757

I saw the red border on the images as soon as I properly fixed the syntax of the code:

 function addBorder() {
      for (var i=0; i < document.getElementsByTagName('img').length; i++) {
           document.getElementsByTagName('img')[i].style.border = "3px solid red";
      }
 }

Upvotes: 0

Related Questions