Made the open menu item in the context menu updated when the context menu is open.

Made the open menu item in the context menu updated when the context menu is open.

Originally, FileSelectionHandler#updateFileSelectionAsync have the complex
conditional statements. And when util.platform.newUI() is true and
selection.totalCount is zero, updateContextMenuActionItems that updates the open
menu item is not called.

This CL arranged the statements by the target of update and added some comments.

BUG=243687
TEST=manually

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202508

[hirono, 2013-05-27 04:09:08]

Yoshiki-san,
Could you check this CL?

Thank you very much!

[yoshiki, 2013-05-28 02:26:48]

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_selection.js (right):

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:481:
selection.createTasks(function() {
Why moved to here? The logic doesn't seem changed.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:482: if
(this.selection != selection) return;
nit: New line after condition.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:499:
selection.computeBytes(function() {
if selection.totalCount == 1, we don't need to compute size of the files.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:500: if
(this.selection != selection) return;
nit: New line after condition.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:516: // Hides the
breadcrumbs list on the root path.
Does this comment appropriate for this block?

Maybe:
// Shows the breadcrumb list when no file is selected and the current directory
is non-root path.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:529: var commands =
this.fileManager_.dialogDom_.querySelectorAll('command');
We don't need to do it when no file is selected.

[hirono, 2013-05-28 04:08:44]

Fixed. Thank you!

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_selection.js (right):

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:481:
selection.createTasks(function() {
On 2013/05/28 02:26:48, yoshiki wrote:
> Why moved to here? The logic doesn't seem changed.

If a callback function is assigned to a variable, the callback function can be
used from everywhere in the outer function.  updateFileSelectionAsync is a bit
large, so, I'd like to show that the callback function is only used here.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:482: if
(this.selection != selection) return;
On 2013/05/28 02:26:48, yoshiki wrote:
> nit: New line after condition.

Done.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:499:
selection.computeBytes(function() {
On 2013/05/28 02:26:48, yoshiki wrote:
> if selection.totalCount == 1, we don't need to compute size of the files. 

Done.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:500: if
(this.selection != selection) return;
On 2013/05/28 02:26:48, yoshiki wrote:
> nit: New line after condition.

Done.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:516: // Hides the
breadcrumbs list on the root path.
On 2013/05/28 02:26:48, yoshiki wrote:
> Does this comment appropriate for this block?
> 
> Maybe:
> // Shows the breadcrumb list when no file is selected and the current
directory
> is non-root path.

Done.

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:529: var commands =
this.fileManager_.dialogDom_.querySelectorAll('command');
On 2013/05/28 02:26:48, yoshiki wrote:
> We don't need to do it when no file is selected.

Done.

[yoshiki, 2013-05-28 04:20:33]

lgtm with nit

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_selection.js (right):

https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:481:
selection.createTasks(function() {
On 2013/05/28 04:08:44, hirono wrote:
> On 2013/05/28 02:26:48, yoshiki wrote:
> > Why moved to here? The logic doesn't seem changed.
> 
> If a callback function is assigned to a variable, the callback function can be
> used from everywhere in the outer function.  updateFileSelectionAsync is a bit
> large, so, I'd like to show that the callback function is only used here.

Got it. Thanks for explanation.

https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_selection.js (right):

https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:493: var
thumbnailEntries = null;
nit: The default value 'null' can't be used. Could you remove it?

[hirono, 2013-05-28 04:46:53]

On 2013/05/28 04:20:33, yoshiki wrote:
> lgtm with nit
> 
>
https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
> File chrome/browser/resources/file_manager/js/file_selection.js (right):
> 
>
https://codereview.chromium.org/15950003/diff/2001/chrome/browser/resources/f...
> chrome/browser/resources/file_manager/js/file_selection.js:481:
> selection.createTasks(function() {
> On 2013/05/28 04:08:44, hirono wrote:
> > On 2013/05/28 02:26:48, yoshiki wrote:
> > > Why moved to here? The logic doesn't seem changed.
> > 
> > If a callback function is assigned to a variable, the callback function can
be
> > used from everywhere in the outer function.  updateFileSelectionAsync is a
bit
> > large, so, I'd like to show that the callback function is only used here.
> 
> Got it. Thanks for explanation.
> 
>
https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
> File chrome/browser/resources/file_manager/js/file_selection.js (right):
> 
>
https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
> chrome/browser/resources/file_manager/js/file_selection.js:493: var
> thumbnailEntries = null;
> nit: The default value 'null' can't be used. Could you remove it?

Fixed.
Thank you very much!

[hirono, 2013-05-28 04:47:01]

https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_selection.js (right):

https://codereview.chromium.org/15950003/diff/7001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_selection.js:493: var
thumbnailEntries = null;
On 2013/05/28 04:20:33, yoshiki wrote:
> nit: The default value 'null' can't be used. Could you remove it?

Done.

[commit-bot: I haz the power, 2013-05-28 04:47:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hirono@chromium.org/15950003/11001

[commit-bot: I haz the power, 2013-05-28 06:59:39]

Change committed as 202508