Mute when clicked on sound button in ChromeOS media player.

Mute when clicked on sound button in ChromeOS media player.

- Add a sound mute icon and swap between sound high and mute icon based on
  volume change;
- Change sound button click to toggle volume mute;
- Show volume control when mouse over sound button and hide it when mouse
  leaves;
- Call volumeChange after we setupPlaybackControls so that volume slider's
  value syncs with media element's volume;
- Removed "controlbutton" class from "volume" element because it is really
  a control button and "controlbutton" style sets its hight to 30px;

BUG=chromium-os:3148
TEST=Verify click on sound button mute/unmute audio and volume slider shows up when mouse over sound button.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49331

[arv (Not doing code reviews), 2010-06-09 18:56:12]

http://codereview.chromium.org/2722006/diff/1/4
File chrome/browser/resources/mediaplayer.html (right):

http://codereview.chromium.org/2722006/diff/1/4#newcode120
chrome/browser/resources/mediaplayer.html:120: background-repeat: no-repeat;
Can you refactor so you don't have to duplicate background-repeat and
background-position?

http://codereview.chromium.org/2722006/diff/1/4#newcode438
chrome/browser/resources/mediaplayer.html:438: var volume_div = $('volume');
No underscores in variable names in JS

http://codereview.chromium.org/2722006/diff/1/4#newcode440
chrome/browser/resources/mediaplayer.html:440: volume_div.style.display =
'block';
volumeDiv.style.display = visible ? 'block' : 'none'

http://codereview.chromium.org/2722006/diff/1/4#newcode453
chrome/browser/resources/mediaplayer.html:453: };
No semicolon for function statements

http://codereview.chromium.org/2722006/diff/1/4#newcode456
chrome/browser/resources/mediaplayer.html:456: if (hideVolumeTimerId == 0)
if (!hideVolumeTimerId)

[xiyuan, 2010-06-09 19:12:27]

http://codereview.chromium.org/2722006/diff/1/4
File chrome/browser/resources/mediaplayer.html (right):

http://codereview.chromium.org/2722006/diff/1/4#newcode120
chrome/browser/resources/mediaplayer.html:120: background-repeat: no-repeat;
On 2010/06/09 18:56:13, arv wrote:
> Can you refactor so you don't have to duplicate background-repeat and
> background-position?

Done.

http://codereview.chromium.org/2722006/diff/1/4#newcode438
chrome/browser/resources/mediaplayer.html:438: var volume_div = $('volume');
On 2010/06/09 18:56:13, arv wrote:
> No underscores in variable names in JS

Done.

http://codereview.chromium.org/2722006/diff/1/4#newcode440
chrome/browser/resources/mediaplayer.html:440: volume_div.style.display =
'block';
On 2010/06/09 18:56:13, arv wrote:
> volumeDiv.style.display = visible ? 'block' : 'none'

Done.

http://codereview.chromium.org/2722006/diff/1/4#newcode453
chrome/browser/resources/mediaplayer.html:453: };
On 2010/06/09 18:56:13, arv wrote:
> No semicolon for function statements
Done and also updated all the other places.

http://codereview.chromium.org/2722006/diff/1/4#newcode456
chrome/browser/resources/mediaplayer.html:456: if (hideVolumeTimerId == 0)
On 2010/06/09 18:56:13, arv wrote:
> if (!hideVolumeTimerId)

Done.

[arv (Not doing code reviews), 2010-06-09 19:17:31]

lgtm but wait for David since he is the owner of this code.

http://codereview.chromium.org/2722006/diff/6001/7003
File chrome/browser/resources/mediaplayer.html (right):

http://codereview.chromium.org/2722006/diff/6001/7003#newcode463
chrome/browser/resources/mediaplayer.html:463: var new_icon = element.volume > 0
? 'soundiconhigh' : 'soundiconmuted';
underscore

http://codereview.chromium.org/2722006/diff/6001/7003#newcode589
chrome/browser/resources/mediaplayer.html:589: volume.onmouseover = showVolume;
This could have been done using CSS

[dhg, 2010-06-09 19:44:54]

LGTM

[xiyuan, 2010-06-09 22:13:55]

http://codereview.chromium.org/2722006/diff/6001/7003
File chrome/browser/resources/mediaplayer.html (right):

http://codereview.chromium.org/2722006/diff/6001/7003#newcode463
chrome/browser/resources/mediaplayer.html:463: var new_icon = element.volume > 0
? 'soundiconhigh' : 'soundiconmuted';
On 2010/06/09 19:17:31, arv wrote:
> underscore

Done.

http://codereview.chromium.org/2722006/diff/6001/7003#newcode589
chrome/browser/resources/mediaplayer.html:589: volume.onmouseover = showVolume;
On 2010/06/09 19:17:31, arv wrote:
> This could have been done using CSS
The reason not using ":hover" etc to do this is because we also need to
show/hide volume on mouse over/out on "soundbutton" element.