Cleanup: Remove unnecessary property: VolumeList.currentVolume_

chrome/browser/resources/file_manager/js/volume_list.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 /**
  * A volume list model. This model combines the 2 lists.
  * @param {cr.ui.ArrayDataModel} volumesList The first list of the model.
  * @param {cr.ui.ArrayDataModel} pinnedList The second list of the model.
@@ skipping 168 matching lines @@
   this.directoryModel_ = directoryModel;
   this.volumeManager_ = VolumeManager.getInstance();
   this.selectionModel = new cr.ui.ListSingleSelectionModel();
 
   this.directoryModel_.addEventListener('directory-changed',
       this.onCurrentDirectoryChanged_.bind(this));
   this.selectionModel.addEventListener(
       'change', this.onSelectionChange_.bind(this));
   this.selectionModel.addEventListener(
       'beforeChange', this.onBeforeSelectionChange_.bind(this));
-  this.currentVolume_ = null;
-
 
   this.scrollBar_ = new ScrollBar();
   this.scrollBar_.initialize(this.parentNode, this);
 
   // Overriding default role 'list' set by cr.ui.List.decorate() to 'listbox'
   // role for better accessibility on ChromeOS.
   this.setAttribute('role', 'listbox');
 
   var self = this;
   this.itemConstructor = function(path) {
@@ skipping 106 matching lines @@
 /**
  * Selects the n-th volume from the list.
  * @param {number} index Volume index.
  * @return {boolean} True for success, otherwise false.
  */
 VolumeList.prototype.selectByIndex = function(index) {
   if (index < 0 || index > this.dataModel.length - 1)
     return false;
 
   var newPath = this.dataModel.item(index);
-  if (!newPath || this.currentVolume_ == newPath)
+  if (!newPath || this.directoryModel.getCurrentDirEntry().fullPath == newPath)

[mtomasz, 2013/08/05 12:17:57]

directoryModel -> directoryModel_

[mtomasz, 2013/08/05 12:17:57]

I think this will not work as it should. getCurrentDirEntry().fullPath will
return the current path, not the current root.

[yoshiki, 2013/08/05 16:08:01]

Done.

[yoshiki, 2013/08/05 16:08:01]

It should work. Previous code was wrong.

Before introducing folder shortcut feature, each root had only an item. If the
selection is changed, the roots of old and new items must be different. But now,
there may be two or more items with same root, so comparing root is not enough.
We need to compare the entire path.

[mtomasz, 2013/08/05 16:25:52]

I see. My concern is that this check was used to avoid reselecting the current
item I guess. Now, it will sometimes allow to reselect and sometimes not. Eg.
when we are in a subdirectory of Drive and then try to change to Drive. This
check will return false and will allow to reselect.

So, I have two questions:
1. Is this check necessary? If not, then can we remove it?
2. Why not checking the selection model if the current selected index == index?

If we can't (1) nor (2) then please just add a comment describing the purpose of
this check.

     return false;
 
-  this.currentVolume_ = newPath;
-  this.directoryModel_.changeDirectory(this.currentVolume_);
+  this.directoryModel_.changeDirectory(newPath);
   return true;
 };
 
 /**
  * Handler before root item change.
  * @param {Event} event The event.
  * @private
  */
 VolumeList.prototype.onBeforeSelectionChange_ = function(event) {
   if (event.changes.length == 1 && !event.changes[0].selected)
@@ skipping 16 matching lines @@
 
 /**
  * Invoked when the current directory is changed.
  * @param {Event} event The event.
  * @private
  */
 VolumeList.prototype.onCurrentDirectoryChanged_ = function(event) {
   var path = event.newDirEntry.fullPath || this.dataModel.getCurrentDirPath();
   var newRootPath = PathUtil.getRootPath(path);
 
-  // Sets |this.currentVolume_| in advance to prevent |onSelectionChange_()|
-  // from calling |DirectoryModel.ChangeDirectory()| again.
-  this.currentVolume_ = newRootPath;
-
   // Synchronizes the volume list selection with the current directory, after
   // it is changed outside of the volume list.
 
   // (1) Select the nearest parent directory (including the pinned directories).
   var bestMatchIndex = -1;
   var bestMatchSubStringLen = 0;
   for (var i = 0; i < this.dataModel.length; i++) {
     var itemPath = this.dataModel.item(i);
     if (path.indexOf(itemPath) == 0) {
       if (bestMatchSubStringLen < itemPath.length) {
@@ skipping 15 matching lines @@
     var itemPath = this.dataModel.item(i);
     if (PathUtil.getRootPath(itemPath) == newRootPath) {
       // Not to invoke the handler of this instance, sets the guard.
       this.dontHandleSelectionEvent_ = true;
       this.selectionModel.selectedIndex = i;
       this.dontHandleSelectionEvent_ = false;
       return;
     }
   }
 };