Sort "related files" navigation order by name in chrome://media-app

Current sort order is by modification time, which means mixed photos
and videos from the camera app always "get older" when pressing right.
However, feedback suggests this is confusing when opening the gallery
from the files app.

Sorting by name instead is more likely to match the sorting field
in the files app, and consistent with what the old gallery app did. The
order defaults to reverse-lexicographic so that pressing "right" still
goes to older files from the camera app.

Bug: b/169012791
Change-Id: I2240ca022519a52f04cd5c1ca1f42f29d014a6ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419099
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808783}

chromeos/components/media_app_ui/resources/js/launch.js

@@ -2,6 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+/**
+ * Sort order for files in the navigation ring.
+ * @enum
+ */
+const SortOrder = {
+  /**
+   * Lexicographic (with natural number ordering): advancing goes "down" the
+   * alphabet.
+   */
+  A_FIRST: 1,
+  /**
+   * Reverse lexicographic (with natural number ordering): advancing goes "up"
+   * the alphabet.
+   */
+  Z_FIRST: 2,
+  /** By modified time: pressing "right" goes to older files. */
+  NEWEST_FIRST: 3,
+};
+
 /**
  * Wrapper around a file handle that allows the privileged context to arbitrate
  * read and write access as well as file navigation. `token` uniquely identifies
@@ -25,6 +44,15 @@ let FileDescriptor;
  */
 const currentFiles = [];
 
+/**
+ * The current sort order.
+ * TODO(crbug/414789): Match the file manager order when launched that way.
+ * Note currently this is reassigned in tests.
+ * @type {!SortOrder}
+ */
+// eslint-disable-next-line prefer-const
+let sortOrder = SortOrder.Z_FIRST;
+
 /**
  * Index into `currentFiles` of the current file.
  *
@@ -670,10 +698,20 @@ async function processOtherFilesInDirectory(
       return -1;
     } else if (!a.file) {
       return 1;
-    } else if (a.file.lastModified === b.file.lastModified) {
-      return a.file.name.localeCompare(b.file.name);
     }
-    return b.file.lastModified - a.file.lastModified;
+    if (sortOrder === SortOrder.NEWEST_FIRST) {
+      if (a.file.lastModified === b.file.lastModified) {
+        return a.file.name.localeCompare(b.file.name);
+      }
+      return b.file.lastModified - a.file.lastModified;
+    }
+    // Match the Intl.Collator params used for sorting in the files app in
+    // file_manager/common/js/util.js.
+    const direction = sortOrder === SortOrder.A_FIRST ? 1 : -1;
+    return direction *
+        a.file.name.localeCompare(
+            b.file.name, [],
+            {usage: 'sort', numeric: true, sensitivity: 'base'});
   });
 
   const name = focusFile.name;

chromeos/components/media_app_ui/test/media_app_ui_browsertest.js

@@ -251,6 +251,7 @@ TEST_F('MediaAppUIBrowserTest', 'ReportsErrorsFromTrustedContext', async () => {
 // MediaApp i.e. doesn't call `launchWithDirectory`, then the rest of the files
 // in the current directory are loaded in.
 TEST_F('MediaAppUIBrowserTest', 'NonLaunchableIpcAfterFastLoad', async () => {
+  sortOrder = SortOrder.A_FIRST;
   const files =
       await createMultipleImageFiles(['file1', 'file2', 'file3', 'file4']);
   const directory = await createMockTestDirectory(files);
@@ -287,6 +288,7 @@ TEST_F('MediaAppUIBrowserTest', 'NonLaunchableIpcAfterFastLoad', async () => {
 // Tests that we can launch the MediaApp with the selected (first) file,
 // and re-launch it before all files from the first launch are loaded in.
 TEST_F('MediaAppUIBrowserTest', 'ReLaunchableAfterFastLoad', async () => {
+  sortOrder = SortOrder.A_FIRST;
   const files =
       await createMultipleImageFiles(['file1', 'file2', 'file3', 'file4']);
   const directory = await createMockTestDirectory(files);
@@ -413,6 +415,7 @@ TEST_F('MediaAppUIBrowserTest', 'LaunchWithUnopenableSibling', async () => {
 // Tests that a file that becomes inaccessible after the initial app launch is
 // ignored on navigation, and shows an error when navigated to itself.
 TEST_F('MediaAppUIBrowserTest', 'NavigateWithUnopenableSibling', async () => {
+  sortOrder = SortOrder.A_FIRST;
   const handles = [
     fileToFileHandle(await createTestImageFile(111 /* width */, 10, '1.png')),
     fileToFileHandle(await createTestImageFile(222 /* width */, 10, '2.png')),
@@ -673,6 +676,7 @@ TEST_F('MediaAppUIBrowserTest', 'DeleteOriginalIPC', async () => {
 // Tests when a file is deleted, the app tries to open the next available file
 // and reloads with those files.
 TEST_F('MediaAppUIBrowserTest', 'DeletionOpensNextFile', async () => {
+  sortOrder = SortOrder.A_FIRST;
   const testFiles = [
     await createTestImageFile(1, 1, 'test_file_1.png'),
     await createTestImageFile(1, 1, 'test_file_2.png'),
@@ -1020,6 +1024,7 @@ TEST_F('MediaAppUIBrowserTest', 'OpenFileIPC', async () => {
 });
 
 TEST_F('MediaAppUIBrowserTest', 'RelatedFiles', async () => {
+  sortOrder = SortOrder.A_FIRST;
   // These files all have a last modified time of 0 so the order they end up in
   // is their lexicographical order i.e. `jaypeg.jpg, jiff.gif, matroska.mkv,
   // world.webm`. When a file is loaded it becomes the "focus file" and files
@@ -1064,7 +1069,8 @@ TEST_F('MediaAppUIBrowserTest', 'RelatedFiles', async () => {
   testDone();
 });
 
-TEST_F('MediaAppUIBrowserTest', 'SortedFiles', async () => {
+TEST_F('MediaAppUIBrowserTest', 'SortedFilesByTime', async () => {
+  sortOrder = SortOrder.NEWEST_FIRST;
   // We want the more recent (i.e. higher timestamp) files first. In the case of
   // equal timestamp, it should sort lexicographically by filename.
   const filesInModifiedOrder = await Promise.all([
@@ -1087,6 +1093,32 @@ TEST_F('MediaAppUIBrowserTest', 'SortedFiles', async () => {
   testDone();
 });
 
+TEST_F('MediaAppUIBrowserTest', 'SortedFilesByName', async () => {
+  // Z_FIRST should be the default.
+  assertEquals(sortOrder, SortOrder.Z_FIRST);
+  // Establish some sample files that match the naming style from the Camera app
+  // in m86, except one file with lowercase prefix is included, to verify that
+  // the collation ignores case (to match the Files app). Note we want
+  // "pressing right" to go to the previously taken photo/video, which means
+  // reverse lexicographic.
+  const filesInReverseLexicographicOrder = await Promise.all([
+    createTestImageFile(1, 1, 'VID_20200921_104848.jpg', 8),  // Video from day.
+    createTestImageFile(1, 1, 'IMG_20200922_104816.jpg', 9),  // Later date.
+    createTestImageFile(1, 1, 'img_20200921_104910.jpg', 6),  // Newest on day.
+    createTestImageFile(1, 1, 'IMG_20200921_104816.jpg', 7),  // Modified.
+    createTestImageFile(1, 1, 'IMG_20200921_104750.jpg', 5),  // Oldest.
+  ]);
+  const files = [...filesInReverseLexicographicOrder];
+  // Mix up files so that we can check they get sorted correctly.
+  [files[4], files[2], files[3]] = [files[2], files[3], files[4]];
+
+  await launchWithFiles(files);
+
+  assertFilesToBe(filesInReverseLexicographicOrder);
+
+  testDone();
+});
+
 // Tests that the guest gets focus automatically on start up.
 TEST_F('MediaAppUIBrowserTest', 'GuestHasFocus', async () => {
   const guest = queryIFrame();