Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am new to javascript and want to make sure I am on the right track when playing a sound. Is there anything I should not be doing better or not be doing at all. Below is a simple function.

<script language="javascript" type="text/javascript">
  function playSound(soundfile) {
    if (navigator.userAgent.indexOf('Firefox') != -1) {
      document.getElementById("dummy").innerHTML = "<embed src=\"" + soundfile + "\" hidden=\"true\" autostart=\"true\" loop=\"false\" type=\"application/x-mplayer2\" />";
    } else {
      document.getElementById("dummy").innerHTML = "<embed src=\"" + soundfile + "\" hidden=\"true\" autostart=\"true\" loop=\"false\" />";
    }
  }
</script>
</head>
<body>
<span id="dummy"></span>

  <a href="#" onclick="playSound('doorbell.wav');">Click here to hear a sound</a>
</body>

I am not sure whether I should be using the audio tag or not. I appreciate any feedback

share|improve this question
    
Why do you use embed instead of audio? –  unor Feb 12 '13 at 9:03
    
Consider building the element using something like jQuery instead of doing a concatenation of strings which is hard to read and difficult to maintain. –  Quentin Pradet Feb 12 '13 at 9:17
add comment

3 Answers

up vote 2 down vote accepted

Don't duplicate sections of code! In particular, avoid putting 'execution' code in these simple conditionals, instead use them to build a shared executor:

function playSound(soundfile) {
  var type_attr = '';
  if (navigator.userAgent.indexOf('Firefox') != -1) {
    type_attr = "type=\"application/x-mplayer2\"";
  }
  document.getElementById("dummy").innerHTML = "<embed src=\"" + soundfile + "\" hidden=\"true\" autostart=\"true\" loop=\"false\" " + type_attr + " />";
}
share|improve this answer
add comment

Regarding the HTML:

script element

For JavaScript, you can simply use <script> without the type attribute:

If the language is not that described by "text/javascript", then the type attribute must be present

The language attribute is obsolete:

Authors should not specify a language attribute on a script element.

a element

I'd say a real button should be used here, not a link:

  • button element with button type: <button type="button" onclick="…">Click here to hear a sound</button>
  • input element with button type: <input type="button" onclick="…" value="Click here to hear a sound" />
share|improve this answer
add comment

Use jQuery and then do it like this:

<pre>
    <script> 
        $(document).ready(function() { 
        var obj = document.createElement("audio"); 
        obj.setAttribute("src", "http://kahimyang.info/resources/click.m... 
        $.get(); 

        $(".playSound").click(function() { 
            obj.play(); 
        }); 
    }); 
    </script> 
</pre>

Put this snippet at the bottom of the page after that all anchors with class "playSound" will play a sound. Somewhere in your page your link should look loke below:

<pre>
    <a href="#" class="playSound">Play</a> 
</pre>

For more details visit this blog:

http://kahimyang.info/kauswagan/code-blogs/1652/how-to-play-a-sound-when-an-element-is-clicked-in-html-page

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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