Commit 247f5c29 authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

[Sharesheet] Fix sharesheet button focus issue.

Currently when we tab through the action bar we cannot focus on the
sharesheet button. This CL fixes this issue and also add a test for this
bug.

The issue with the original implementation is because we disable the
command when we are waiting for feedback from the C++ api, however when
we tab through the action bar, the canExecute() is called and we cannot
focus on the sharesheet button while tabing because the command is still
disabled at the time. Changing the code to keep the command enable if
the event target is action bar. This should be safe because in this
event, there should be no change to the selected entry so if the button
is visible at the time, it should be safe to be enabled.

BUG=1136334, 1097623

Change-Id: I285bb7fe71322c05ae660fb02917126e7491b17a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460854Reviewed-by: default avatarLuciano Pacheco <lucmult@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816063}
parent 3bf12e46
......@@ -130,6 +130,11 @@ struct TestCase {
return *this;
}
TestCase& DisableSharesheet() {
options.enable_sharesheet = false;
return *this;
}
std::string GetFullName() const {
std::string full_name = name;
......@@ -788,8 +793,15 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
TestCase("tabindexFocusDownloads").InGuestMode().DisableFilesNg(),
// TestCase("tabindexFocusBreadcrumbBackground").FilesNg(),
TestCase("tabindexFocusBreadcrumbBackground").DisableFilesNg(),
TestCase("tabindexFocusDirectorySelected").FilesNg(),
TestCase("tabindexFocusDirectorySelected").DisableFilesNg(),
TestCase("tabindexFocusDirectorySelected")
.FilesNg()
.DisableSharesheet(),
TestCase("tabindexFocusDirectorySelected")
.DisableFilesNg()
.DisableSharesheet(),
TestCase("tabindexFocusDirectorySelectedSharesheetEnabled")
.FilesNg()
.EnableSharesheet(),
TestCase("tabindexOpenDialogDownloadsFilesNg").WithBrowser().FilesNg(),
TestCase("tabindexOpenDialogDownloads").WithBrowser().DisableFilesNg(),
TestCase("tabindexOpenDialogDownloads")
......
......@@ -1641,6 +1641,8 @@ void FileManagerBrowserTestBase::SetUpCommandLine(
if (options.enable_sharesheet) {
enabled_features.push_back(features::kSharesheet);
} else {
disabled_features.push_back(features::kSharesheet);
}
if (options.single_partition_format) {
......
......@@ -1672,7 +1672,12 @@ CommandHandler.COMMANDS_['invoke-sharesheet'] = new class extends Command {
}
event.canExecute = true;
event.command.disabled = true;
// In the case where changing focus to action bar elements, it is safe to
// keep the command enabled if it was visible before, because there should
// be no change to the selected entries.
event.command.disabled =
!fileManager.ui.actionbar.contains(/** @type {Node} */ (event.target));
chrome.fileManagerPrivate.sharesheetHasTargets(entries, hasTargets => {
if (chrome.runtime.lastError) {
console.error(chrome.runtime.lastError.message);
......
......@@ -231,6 +231,73 @@ testcase.tabindexFocusDirectorySelected = async () => {
}
};
/**
* Tests the tab focus behavior of the Files app when a directory is selected.
*/
testcase.tabindexFocusDirectorySelectedSharesheetEnabled = async () => {
// Open Files app on Drive.
const appId = await setupAndWaitUntilReady(RootPath.DRIVE);
// Check that the file list has the focus on launch.
await Promise.all([
remoteCall.waitForElement(appId, ['#file-list:focus']),
remoteCall.waitForElement(appId, ['#drive-welcome-link']),
]);
const element =
await remoteCall.callRemoteTestUtil('getActiveElement', appId, []);
chrome.test.assertEq('list', element.attributes['class']);
// Fake chrome.fileManagerPrivate.sharesheetHasTargets to return true.
const fakeData = {
'chrome.fileManagerPrivate.sharesheetHasTargets': ['static_fake', [true]],
};
await remoteCall.callRemoteTestUtil('foregroundFake', appId, [fakeData]);
// Select the directory named 'photos'.
chrome.test.assertTrue(
await remoteCall.callRemoteTestUtil('selectFile', appId, ['photos']));
await Promise.all([
// Wait for share button to to be visible and enabled.
remoteCall.waitForElement(
appId, ['#sharesheet-button:not([hidden]):not([disabled])']),
// Wait for delete button to to be visible and enabled.
remoteCall.waitForElement(
appId, ['#delete-button:not([hidden]):not([disabled])']),
]);
// Send Tab key events to cycle through the tabable elements.
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'directory-tree'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'pinned-toggle'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'sharesheet-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'delete-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'search-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'view-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'sort-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'gear-button'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'drive-welcome-link'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'welcome-dismiss'));
chrome.test.assertTrue(
await remoteCall.checkNextTabFocus(appId, 'file-list'));
// Remove fakes.
const removedCount = await remoteCall.callRemoteTestUtil(
'removeAllForegroundFakes', appId, []);
chrome.test.assertEq(1, removedCount);
};
/**
* Tests the tab focus in the dialog and closes the dialog.
*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment