[AudioPlayer] Fix a bug on changing 'expanded' status

chrome/browser/resources/file_manager/audio_player/elements/track_list.js

Index: chrome/browser/resources/file_manager/audio_player/elements/track_list.js
diff --git a/chrome/browser/resources/file_manager/audio_player/elements/track_list.js b/chrome/browser/resources/file_manager/audio_player/elements/track_list.js
index 6b373ad6da5d7a6ad960fc101738c22967f0b175..5491d7d262b7a5bcf1689706552f9e5b11cd8fcb 100644
--- a/chrome/browser/resources/file_manager/audio_player/elements/track_list.js
+++ b/chrome/browser/resources/file_manager/audio_player/elements/track_list.js
@@ -154,10 +154,22 @@
       var trackSelector = '.track[index="' + trackIndex + '"]';
       var trackElement = this.impl.querySelector(trackSelector);
       if (trackElement) {
-        this.scrollTop = Math.max(
-            0,
-            (trackElement.offsetTop + trackElement.offsetHeight -
-             this.clientHeight));
+        var viewTop = this.scrollTop;
+        var viewHeight = this.clientHeight;
+        var elementTop = trackElement.offsetTop;
+        var elementHeight = trackElement.offsetHeight;
+
+        if (elementTop < viewTop) {
+          // Adjust the tops.
+          this.scrollTop = elementTop;
+        } else if (viewTop <= elementTop &&

[hirono, 2014/03/05 17:24:01]

nit: The first condition check can be removed.

[yoshiki, 2014/03/06 08:25:07]

Done.

+                   elementTop + elementHeight <= viewTop + viewHeight) {

[hirono, 2014/03/05 17:24:01]

nit: How about negating the condition and put the fix of bottom coordinates in
the second block?

[yoshiki, 2014/03/05 18:30:41]

I think it's better for readability, specifying we have 3 conditions of the
track position: (1) the target track is above the viewport, (2) it is in the
viewport, and (3) it is below the viewport.

[hirono, 2014/03/06 03:44:47]

One point I cared a bit is the condition check is repeated at #165 to show the
condition explicitly but is not repeated at #168.
Another is (1) and (3) are not exclusive as long as we don't know the viewport
height is never less than the element height.
But I don't have a strong opinion here. The current code is still l g t m.

+          // The entire element is in the viewport. Do nothing.
+        } else {
+          // Adjust the bottoms.
+          this.scrollTop = Math.max(0,
+                                    (elementTop + elementHeight - viewHeight));
+        }
       }
     },