Code review comment for lp:~xerosis/nuvola-player/squeezebox-integration

Here is a review:

----8<----

6 +<p>The <strong>Squeezebox</strong> is a network music player from Logitech. It plays internet radio or digital ...

This line is too long, wrap it to 80 characters.

----8<----

40 + * @param Nuvola Nuvola JS API

This line can be removed (function does not have any arguments)

----8<----

60 + var state = Nuvola.STATE_PAUSED;

Default state should be Nuvola.STATE_NONE

----8<----

72 + var album_art = document.getElementById('ctrlCurrentArt').firstChild.firstChild.src;
73 + var artist = document.getElementById('ctrlCurrentArtist').lastChild.innerText;
74 + var song = document.getElementById('ctrlCurrentTitle').firstChild.innerText;
75 + // Squeezebox prepends the song number to the song name, remove this
76 + var song = song.split(/\.(.+)?/)[1].trim();
77 + var album = document.getElementById('ctrlCurrentAlbum').firstChild.innerText;

No need to use "var" since all variables are already declared at the beginning of the function.

----8<----

83 + var play_pause = document.getElementById('ext-gen42');
84 + var pp_pressed = play_pause.getAttribute("title");
85 + var state;
86 + if(pp_pressed == "Pause"){
87 + state = Nuvola.STATE_PLAYING;
88 + }
89 + else{
90 + state = Nuvola.STATE_PAUSED;
91 + }

If play_pause is null, line 84 raises exception. This is better:

var play_pause = document.getElementById('ext-gen42');
try{
   var pp_pressed = play_pause.getAttribute("title");
   state = (pp_pressed == "Pause") ? Nuvola.STATE_PLAYING : Nuvola.STATE_PAUSED;
}
catch(e){
// state stays Nuvola.STATE_NONE
}