Files app: Fix context menu for renaming files/folders.

Naming/Renaming input commits the new name when the input loses focus,
this is broken when user right-click on the input, since context menu
pulls the focus to itself for a11y.

Change the renaming controllers to listen to the contexmenu event to
be able to not commit the new name in this condition.

Change some "let" to "const" as pointed by presubmit.

Fixed: 1008816
Change-Id: Id5ccb97598afd597871793d2165f3132b9363448
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049150Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Auto-Submit: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740544}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -405,6 +405,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("checkRenameDisabledForReadOnlyDocument"),
         TestCase("checkRenameDisabledForReadOnlyFile"),
         TestCase("checkRenameDisabledForReadOnlyFolder"),
+        TestCase("checkContextMenuForRenameInput"),
         TestCase("checkShareEnabledForReadWriteFile"),
         TestCase("checkShareEnabledForReadOnlyDocument"),
         TestCase("checkShareDisabledForStrictReadOnlyDocument"),
@@ -546,6 +547,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("dirRenameRemovableWithKeyboard").InGuestMode(),
         TestCase("dirRenameRemovableWithContentMenu"),
         TestCase("dirRenameRemovableWithContentMenu").InGuestMode(),
+        TestCase("dirContextMenuForRenameInput"),
         TestCase("dirCreateWithContextMenu"),
         TestCase("dirCreateWithKeyboard"),
         TestCase("dirCreateWithoutChangingCurrent"),

ui/file_manager/file_manager/foreground/js/directory_tree_naming_controller.js

@@ -33,6 +33,13 @@ class DirectoryTreeNamingController {
     /** @private {?VolumeInfo} */
     this.volumeInfo_ = null;
 
+    /**
+     * Controls if a context menu is shown for the rename input, so it shouldn't
+     * commit the new name.
+     * @private {boolean}
+     */
+    this.showingContextMenu_ = false;
+
     /** @private @const {!HTMLInputElement} */
     this.inputElement_ = /** @type {!HTMLInputElement} */
         (document.createElement('input'));
@@ -45,6 +52,9 @@ class DirectoryTreeNamingController {
       // directory item and current directory is changed to editing item.
       event.stopPropagation();
     });
+    this.inputElement_.addEventListener(
+        'contextmenu', this.onContextMenu_.bind(this));
+    this.inputElement_.addEventListener('focus', this.onFocus_.bind(this));
   }
 
   /**
@@ -105,12 +115,22 @@ class DirectoryTreeNamingController {
     this.editing_ = true;
   }
 
+  /** @private */
+  onContextMenu_() {
+    this.showingContextMenu_ = true;
+  }
+
+  /** @private */
+  onFocus_() {
+    this.showingContextMenu_ = false;
+  }
+
   /**
    * Commits rename.
    * @private
    */
   commitRename_() {
-    if (!this.editing_) {
+    if (!this.editing_ || this.showingContextMenu_) {
       return;
     }
     this.editing_ = false;

ui/file_manager/file_manager/foreground/js/naming_controller.js

@@ -35,11 +35,22 @@ class NamingController {
     /** @private @const {!FileSelectionHandler} */
     this.selectionHandler_ = selectionHandler;
 
+    /**
+     * Controls if a context menu is shown for the rename input, so it shouldn't
+     * commit the new name.
+     * @private {boolean}
+     */
+    this.showingContextMenu_ = false;
+
     // Register events.
     this.listContainer_.renameInput.addEventListener(
         'keydown', this.onRenameInputKeyDown_.bind(this));
     this.listContainer_.renameInput.addEventListener(
         'blur', this.onRenameInputBlur_.bind(this));
+    this.listContainer_.renameInput.addEventListener(
+        'contextmenu', this.onContextMenu_.bind(this));
+    this.listContainer_.renameInput.addEventListener(
+        'focus', this.onFocus_.bind(this));
   }
 
   /**
@@ -226,11 +237,23 @@ class NamingController {
     }
   }
 
+  onContextMenu_(event) {
+    this.showingContextMenu_ = true;
+  }
+
+  onFocus_(event) {
+    this.showingContextMenu_ = false;
+  }
+
   /**
    * @param {Event} event Blur event.
    * @private
    */
   onRenameInputBlur_(event) {
+    if (this.showingContextMenu_) {
+      return;
+    }
+
     if (this.isRenamingInProgress() &&
         !this.listContainer_.renameInput.validation_) {
       this.commitRename_();

ui/file_manager/integration_tests/file_manager/context_menu.js

@@ -387,7 +387,7 @@ testcase.checkContextMenusForInputElements = async () => {
 
   // Focus the search box.
   chrome.test.assertEq(2, elements.length);
-  for (let element of elements) {
+  for (const element of elements) {
     chrome.test.assertEq('#text-context-menu', element.attributes.contextmenu);
   }
 
@@ -417,6 +417,55 @@ testcase.checkContextMenusForInputElements = async () => {
   await remoteCall.waitForElement(appId, '#text-context-menu:not([hidden])');
 };
 
+/**
+ * Tests that opening context menu in the rename input won't commit the
+ * renaming.
+ */
+testcase.checkContextMenuForRenameInput = async () => {
+  const textInput = '#file-list .table-row[renaming] input.rename';
+  const contextMenu = '#text-context-menu:not([hidden])';
+
+  // Open FilesApp on Downloads.
+  const appId = await setupAndWaitUntilReady(RootPath.DOWNLOADS);
+
+  // Select the file.
+  chrome.test.assertTrue(
+      await remoteCall.callRemoteTestUtil('selectFile', appId, ['hello.txt']),
+      'selectFile failed');
+
+  // Press Ctrl+Enter key to rename the file.
+  const key = ['#file-list', 'Enter', true, false, false];
+  chrome.test.assertTrue(
+      await remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key));
+
+  // Check: the renaming text input should be shown in the file list.
+  await remoteCall.waitForElement(appId, textInput);
+
+  // Type new file name.
+  await remoteCall.callRemoteTestUtil(
+      'inputText', appId, [textInput, 'NEW NAME']);
+
+  // Right click to show the context menu.
+  await remoteCall.waitAndRightClick(appId, textInput);
+
+  // Context menu must be visible.
+  await remoteCall.waitForElement(appId, contextMenu);
+
+  // Dismiss the context menu.
+  const escKey = [contextMenu, 'Escape', false, false, false];
+  await remoteCall.callRemoteTestUtil('fakeKeyDown', appId, escKey);
+
+  // Check: The rename input should be still be visible and with the same
+  // content.
+  const inputElement = await remoteCall.waitForElement(appId, textInput);
+  chrome.test.assertEq('NEW NAME', inputElement.value);
+
+  // Check: The rename input should be the focused element.
+  const focusedElement =
+      await remoteCall.callRemoteTestUtil('getActiveElement', appId, []);
+  chrome.test.assertEq(inputElement, focusedElement);
+};
+
 /**
  * Tests that the specified menu item is in |expectedEnabledState| when the
  * context menu is opened from the file list inside the folder called
@@ -528,7 +577,7 @@ async function checkMyFilesRootItemContextMenu(itemName, commandStates) {
 
   const enabledCmds = [];
   const disabledCmds = [];
-  for (let [cmd, enabled] of Object.entries(commandStates)) {
+  for (const [cmd, enabled] of Object.entries(commandStates)) {
     chrome.test.assertTrue(cmd in validCmds, cmd + ' is not a valid command.');
     if (enabled) {
       enabledCmds.push(cmd);
@@ -570,14 +619,14 @@ async function checkMyFilesRootItemContextMenu(itemName, commandStates) {
 
   // Check the enabled commands.
   for (const commandId of enabledCmds) {
-    let query = `#file-context-menu:not([hidden]) [command="#${
+    const query = `#file-context-menu:not([hidden]) [command="#${
         commandId}"]:not([disabled])`;
     await remoteCall.waitForElement(appId, query);
   }
 
   // Check the disabled commands.
   for (const commandId of disabledCmds) {
-    let query =
+    const query =
         `#file-context-menu:not([hidden]) [command="#${commandId}"][disabled]`;
     await remoteCall.waitForElement(appId, query);
   }

ui/file_manager/integration_tests/file_manager/directory_tree_context_menu.js

@@ -655,6 +655,51 @@
     return IGNORE_APP_ERRORS;
   };
 
+  /**
+   * Tests that opening context menu in the rename input won't commit the
+   * renaming.
+   */
+  testcase.dirContextMenuForRenameInput = async () => {
+    // Open Files app on local downloads.
+    const appId =
+        await setupAndWaitUntilReady(RootPath.DOWNLOADS, [ENTRIES.photos], []);
+
+    // Navigate to the photos folder.
+    navigateWithDirectoryTree(appId, '/My files/Downloads/photos');
+
+    // Start renaming the photos folder.
+    clickDirectoryTreeContextMenuItem(appId, '/Downloads/photos', 'rename');
+
+    // Check: the renaming text input element should appear.
+    const textInput = '#directory-tree .tree-row[selected] input';
+    await remoteCall.waitForElement(appId, textInput);
+
+    // Type new file name.
+    await remoteCall.callRemoteTestUtil(
+        'inputText', appId, [textInput, 'NEW NAME']);
+
+    // Right click to show the context menu.
+    await remoteCall.waitAndRightClick(appId, textInput);
+
+    // Context menu must be visible.
+    const contextMenu = '#text-context-menu:not([hidden])';
+    await remoteCall.waitForElement(appId, contextMenu);
+
+    // Dismiss the context menu.
+    const escKey = [contextMenu, 'Escape', false, false, false];
+    await remoteCall.callRemoteTestUtil('fakeKeyDown', appId, escKey);
+
+    // Check: The rename input should be still be visible and with the same
+    // content.
+    const inputElement = await remoteCall.waitForElement(appId, textInput);
+    chrome.test.assertEq('NEW NAME', inputElement.value);
+
+    // Check: The rename input should be the focused element.
+    const focusedElement =
+        await remoteCall.callRemoteTestUtil('getActiveElement', appId, []);
+    chrome.test.assertEq(inputElement, focusedElement);
+  };
+
   /**
    * Tests creating a folder with the context menu.
    */