12

I have a fairly simple page with a sidebar nav and an iFrame in which to load content.

I want to change the content of the of the iFrame by clicking on links in the sidebar nav. I'm using the javascript to change the source (.src) of the document.element.

I've read numerous articles (StackOverflow) and the code should work, but is simply doesn't.

The code below is the html page I've created and it includes the JS in a tag.

<head>
<meta charset="UTF-8">
<title>Untitled Document</title>
<link href="css/default.css" rel="stylesheet" type="text/css">

<script type="text/javascript">
    function getContent{

        document.getElementById("contentFrame").src="LoremIpsum.html";  

    }
    </script>


</head>

<body>
<div class="sidebar"><h1>Technical Documentation</h1>
<ul>
    <li>Configuration Guides</li>
    <li>API Guides</li>
    <li><a href="" onclick='getContent()'> LoremIpsum</a></li>
</ul>
<!-- <button onclick="getContent()">Lorem Ipsum</button>    -->

</div>


<iframe class="content"  id="contentFrame" src="dummy.html">
</iframe>

</body>
2
  • 3
    Try adding parentheses to the js function declaration. Instead of 'function getContent{' use 'function getContent() {'. Commented Apr 17, 2017 at 21:30
  • Look at the console, and you will see that you have a syntax error Commented Apr 17, 2017 at 21:31

4 Answers 4

29

Your problem was that you forgot to add () after your function name.

Beyond that, there are a few other things to correct:

Don't use inline HTML event attributes (onclick, onmouseover, etc.) as they:

  1. Create "spaghetti code" that is difficult to read and debug.
  2. Lead to duplication of code.
  3. Don't scale well
  4. Don't follow the separation of concerns development methodology.
  5. Create anonymous global wrapper functions around your attribute values that alter the this binding in your callback functions.
  6. Don't follow the W3C Event Standard.
  7. Don't cause a reference to the DOM event to be passed to the handler.

Even MDN agrees

Inline event handlers — don't use these

You might also see a pattern like this in your code:

<button onclick="bgChange()">Press me</button> 

function bgChange() {   
  const rndCol = `rgb(${random(255)}, ${random(255)}, ${random(255)})`;  
  document.body.style.backgroundColor = rndCol; 
} 

The earliest method of registering event handlers found on the Web involved event handler HTML attributes (or inline event handlers) like the one shown above — the attribute value is literally the JavaScript code you want to run when the event occurs. The above example invokes a function defined inside a element on the same page, but you could also insert JavaScript directly inside the attribute, for example:

 <button onclick="alert('This is my old-fashioned event handler!');">Press me</button> 

You can find HTML attribute equivalents for many of the event handler properties; however, you shouldn't use these — they are considered bad practice. It might seem easy to use an event handler attribute if you are doing something really quick, but they quickly become unmanageable and inefficient.

For a start, it is not a good idea to mix up your HTML and your JavaScript, as it becomes hard to read. Keeping your JavaScript separate is a good practice, and if it is in a separate file you can apply it to multiple HTML documents.

Even in a single file, inline event handlers are not a good idea. One button is OK, but what if you had 100 buttons? You'd have to add 100 attributes to the file; it would quickly turn into a maintenance nightmare. With JavaScript, you could easily add an event handler function to all the buttons on the page no matter how many there were, using something like this:

const buttons = document.querySelectorAll("button");
 
for (const button of buttons) {   
  button.addEventListener("click", bgChange); 
} 

Finally, many common server configurations will disallow inline JavaScript, as a security measure.

You should never use the HTML event handler attributes — those are outdated, and using them is bad practice.

Instead, do all your work in JavaScript and use .addEventListener() to set up event handlers.


Don't use a hyperlink when a button (or some other element) will do. If you do use a hyperlink, you need to disable its native desire to navigate, which is done by setting the href attribute to #, not "".

// Place this code into a <script> element that goes just before the closing body tag (</body>).

// Get a reference to the button and the iframe
var btn = document.getElementById("btnChangeSrc");
var iFrame = document.getElementById("contentFrame");

// Set up a click event handler for the button
btn.addEventListener("click", getContent);

function getContent() {
  console.log("Old source was: " +  iFrame.src);
  iFrame.src="LoremIpsum.html";  
  console.log("New source is: " +  iFrame.src);
}
<div class="sidebar"><h1>Technical Documentation</h1>
<ul>
    <li>Configuration Guides</li>
    <li>API Guides</li>
</ul>
<button id="btnChangeSrc">Change Source</button>

</div>

<iframe class="content" id="contentFrame" src="dummy.html"></iframe>

Sign up to request clarification or add additional context in comments.

28 Comments

If using inline HTML event attributes is so bad - then tell me: why all modern frameworks like angular, react, vue etc... are designed to use inline events in their HTML templates? For small pure JS projects using inline HTML events is acceptable (because their are small and simple, and easy to control), for bigger project is better to use some modern framework instead pure js.
@KamilKiełczewski Your point has been brought up over and over again in many forums. It stems from a fundamental misunderstanding and false equivalency. A framework has to have "hooks" into the native code, so they come up with inline ways of doing that, but the rendered code does not contain these inline hooks. I am not discussing frameworks, I am discussing native code that doesn't get transformed into anything else. Your statement that small projects make inline events OK is not a "thing". Inline event handlers are bad... at any scale.
Well, that's your opinion and you are entitled to it. After teaching this stuff since it was invented and some 20,000+ students later, my opinion is that as long as you understand what a framework is and how it accomplishes what it does, there is no problem at all in understanding that coding a framework takes a different approach than coding in native JS. The bad habit that I see is poor training that skips over this topic and dives right into coding without differentiating between the two. It's not rocket science to understand best practices to each.
And, as my post above lays out, this is not just a matter of style or preference. There are real negative implications for your code in JS if you use inline event handlers - - that's the real reason to stay away from them. The very bad habits you talk about truly do manifest in JS itself when you use them.
This is one of the funniest comment threads on stack overflow. The reasons given for not using inline HTML attributes — even by MDN — all boil down to "well what if you need it 100 times?" Which is great, but… I don't. I want to run a specific piece of Javascript on a specific page when a specific form is submitted. There is no scale, there's no duplication, and all that making the event handler added on separately in javascript would accomplish is make the code less readable, because then I have to search all included files for what's affecting the form submission. I'm sticking with inline.
|
2

This is a syntax error. Parenthesis are required no matter the parameters of the function. However, it is a good practice to place script tags at the bottom of the body tag.

Comments

1

You have a syntax error in your function declaration (missing parenthesis):

function getContent {
   document.getElementById("contentFrame").src="LoremIpsum.html";
}

Should be:

function getContent() {
   document.getElementById("contentFrame").src="LoremIpsum.html";
}

You also need to prevent the default event for the link

<head>
<meta charset="UTF-8">
<title>Untitled Document</title>
<link href="css/default.css" rel="stylesheet" type="text/css">

<script type="text/javascript">
    function getContent(event) {
       event.preventDefault();
       document.getElementById("contentFrame").src="LoremIpsum.html";
    }
    </script>


</head>

<body>
<div class="sidebar"><h1>Technical Documentation</h1>
<ul>
    <li>Configuration Guides</li>
    <li>API Guides</li>
    <li><a href="#" onclick='getContent(event)'> LoremIpsum</a></li>
</ul>
<!-- <button onclick="getContent()">Lorem Ipsum</button>    -->

</div>


<iframe class="content"  id="contentFrame" src="dummy.html">
</iframe>

</body>

7 Comments

To prevent the link navigation, you can just set the href to #. No JavaScript needed.
Fair enough, I tend to steer clear of inlining javascript like this and prefer to use event.preventDefault()
Setting the value to # is not inlining JavaScript. There's no JavaScript at all with that.
I'm aware of that, I was referring to onclick='getContent(event)'
Yes, agreed (and I discuss that in detail in my answer). But, your reply to my comment was not about what my comment was about in the first place.
|
0

You can use javascript:void(0).

    <a href="javascript:void(0)" onclick='getContent(event)'> LoremIpsum</a>

Or return false on javascript

     <script type="text/javascript">
function getContent(){

    document.getElementById("contentFrame").src="LoremIpsum.html";
    return false;

}
</script>

1 Comment

Oh no, please don't advocate javascript:void(0) (it's not 1997)! Also, if you were going to do this, adding return false to the function wouldn't be enough. You'd also need the onclick attribute to look like this: onclick="return getContent()" and the event argument wouldn't be passed to getContent, it would wind up being passed to the anonymous global wrapper function that wraps it. See my answer for more details on why this is bad.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.