brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.63k stars 2.3k forks source link

Improve sorting of items in the file selector window #25884

Open vannevar-morgan opened 1 year ago

vannevar-morgan commented 1 year ago

Description

In the file selector window you can choose to sort by e.g., Name / Size / Type / Modified. This sort criteria is applied to all items in the current directory but this makes it difficult to navigate to the correct directory. It's more desirable to apply the sort criteria independently on files and directories.

Steps to Reproduce

  1. Go to a site or form requiring file upload (e.g., https://github.com/settings/profile >> on profile picture, click "Edit", click "Upload a photo...")
  2. In the file selector window, navigate to any directory on your local system with a mix of files and subdirectories (it helps show how unwieldy the current behavior is if there are many files + directories e.g., 50+)
  3. The sort criteria is applied to all items, but it's probably desirable for navigation that directories be grouped and sorted independently of all other files.

Actual result:

Files and directories are jumbled together by the sort criteria. Makes navigation difficult.

Expected result:

Files should be grouped together, directories should be grouped together (typically, directories shown at the top of the file list, files at the bottom). Sort criteria should be applied on each group. Files and directories should not be mixed in order when shown in the file selection window.

Reproduces how often:

Always.

Brave version (brave://version info)

Brave | 1.44.105 Chromium: 106.0.5249.91 (Official Build) (64-bit) Revision | fa96d5f07b1177d1bf5009f647a5b8c629762157-refs/branch-heads/5249@{#707} OS | Linux

Version/Channel Information:

I haven't tested other channels / releases.

Other Additional Information:

Miscellaneous Information:

I've only tested this on Ubuntu 22.04.1 LTS.

There's a related issue with the file selection window - switching back and forth between different sort criteria (file name / file type) sometimes causes the file selection window to crash. I don't have exact steps to reproduce this but you may run into that crash while investigating this issue.

vannevar-morgan commented 1 year ago

This problem is upstream in chrome. https://github.com/chromium/chromium/blob/main/net/base/directory_lister.cc

void SortData(std::vector<DirectoryLister::DirectoryListerData>* data,
              DirectoryLister::ListingType listing_type) {
  // Sort the results. See the TODO below (this sort should be removed and we
  // should do it from JS).
  if (listing_type == DirectoryLister::ALPHA_DIRS_FIRST) {
    std::sort(data->begin(), data->end(), CompareAlphaDirsFirst);
  } else if (listing_type != DirectoryLister::NO_SORT &&
             listing_type != DirectoryLister::NO_SORT_RECURSIVE) {
    NOTREACHED();
  }
}

https://github.com/chromium/chromium/blob/main/chrome/browser/file_select_helper.cc

void FileSelectHelper::StartNewEnumeration(const base::FilePath& path) {
  base_dir_ = path;
  auto entry = std::make_unique<ActiveDirectoryEnumeration>(path);
  entry->lister_ = base::WrapUnique(new net::DirectoryLister(
      path, net::DirectoryLister::NO_SORT_RECURSIVE, this));
  entry->lister_->Start();
  directory_enumeration_ = std::move(entry);
}

It's probably sufficient to change net::DirectoryLister::NO_SORT_RECURSIVE to net::DirectoryLister::ALPHA_DIRS_FIRST

I'm not a chrome developer and the burden for getting a commit in is onerous for a one line change. I submitted a pull request anyway for visibility but will be rejected for not following the checklist. https://github.com/chromium/chromium/pull/137

If someone who is a chrome developer can make the change, it's an easy change to make and test.