Remove credential sorting from Browser Integration (#6353)

src/browser/BrowserService.cpp

@@ -750,14 +750,9 @@ BrowserService::sortEntries(QList<Entry*>& pwEntries, const QString& siteUrlStr,
     std::sort(keys.begin(), keys.end(), [](int l, int r) { return l > r; });
 
     QList<Entry*> results;
-    auto sortField = browserSettings()->sortByTitle() ? EntryAttributes::TitleKey : EntryAttributes::UserNameKey;
     for (auto key : keys) {
-        // Sort same priority entries by Title or UserName
+        results << priorities.values(key);
-        auto entries = priorities.values(key);
+
-        std::sort(entries.begin(), entries.end(), [&sortField](Entry* left, Entry* right) {
-            return QString::localeAwareCompare(left->attribute(sortField), right->attribute(sortField)) < 0;
-        });
-        results << entries;
         if (browserSettings()->bestMatchOnly() && !results.isEmpty()) {
             // Early out once we find the highest batch of matches
             break;

src/browser/BrowserSettings.cpp

@@ -82,26 +82,6 @@ void BrowserSettings::setMatchUrlScheme(bool matchUrlScheme)
     config()->set(Config::Browser_MatchUrlScheme, matchUrlScheme);
 }
 
-bool BrowserSettings::sortByUsername()
-{
-    return config()->get(Config::Browser_SortByUsername).toBool();
-}
-
-void BrowserSettings::setSortByUsername(bool sortByUsername)
-{
-    config()->set(Config::Browser_SortByUsername, sortByUsername);
-}
-
-bool BrowserSettings::sortByTitle()
-{
-    return !sortByUsername();
-}
-
-void BrowserSettings::setSortByTitle(bool sortByUsertitle)
-{
-    setSortByUsername(!sortByUsertitle);
-}
-
 bool BrowserSettings::alwaysAllowAccess()
 {
     return config()->get(Config::Browser_AlwaysAllowAccess).toBool();

src/browser/BrowserSettings.h

@@ -42,10 +42,6 @@ public:
     void setUnlockDatabase(bool unlockDatabase);
     bool matchUrlScheme();
     void setMatchUrlScheme(bool matchUrlScheme);
-    bool sortByUsername();
-    void setSortByUsername(bool sortByUsername = true);
-    bool sortByTitle();
-    void setSortByTitle(bool sortByUsertitle = true);
     bool alwaysAllowAccess();
     void setAlwaysAllowAccess(bool alwaysAllowAccess);
     bool alwaysAllowUpdate();

src/browser/BrowserSettingsWidget.cpp

@@ -117,12 +117,6 @@ void BrowserSettingsWidget::loadSettings()
     // TODO: fix this
     m_ui->showNotification->hide();
 
-    if (settings->sortByUsername()) {
-        m_ui->sortByUsername->setChecked(true);
-    } else {
-        m_ui->sortByTitle->setChecked(true);
-    }
-
     m_ui->alwaysAllowAccess->setChecked(settings->alwaysAllowAccess());
     m_ui->alwaysAllowUpdate->setChecked(settings->alwaysAllowUpdate());
     m_ui->httpAuthPermission->setChecked(settings->httpAuthPermission());
@@ -212,7 +206,6 @@ void BrowserSettingsWidget::saveSettings()
     settings->setBestMatchOnly(m_ui->bestMatchOnly->isChecked());
     settings->setUnlockDatabase(m_ui->unlockDatabase->isChecked());
     settings->setMatchUrlScheme(m_ui->matchUrlScheme->isChecked());
-    settings->setSortByUsername(m_ui->sortByUsername->isChecked());
 
     settings->setUseCustomProxy(m_ui->useCustomProxy->isChecked());
     settings->setCustomProxyLocation(m_ui->customProxyLocation->text());

src/browser/BrowserSettingsWidget.ui

@@ -246,20 +246,6 @@
          </property>
         </widget>
        </item>
-       <item>
-        <widget class="QRadioButton" name="sortByTitle">
-         <property name="text">
-          <string extracomment="Credentials mean login data requested via browser extension">Sort matching credentials by title</string>
-         </property>
-        </widget>
-       </item>
-       <item>
-        <widget class="QRadioButton" name="sortByUsername">
-         <property name="text">
-          <string extracomment="Credentials mean login data requested via browser extension">Sort matching credentials by username</string>
-         </property>
-        </widget>
-       </item>
        <item>
         <spacer name="verticalSpacer_2">
          <property name="orientation">

src/core/Config.cpp

@@ -147,7 +147,6 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
     {Config::Browser_BestMatchOnly, {QS("Browser/BestMatchOnly"), Roaming, false}},
     {Config::Browser_UnlockDatabase, {QS("Browser/UnlockDatabase"), Roaming, true}},
     {Config::Browser_MatchUrlScheme, {QS("Browser/MatchUrlScheme"), Roaming, true}},
-    {Config::Browser_SortByUsername, {QS("Browser/SortByUsername"), Roaming, false}},
     {Config::Browser_SupportBrowserProxy, {QS("Browser/SupportBrowserProxy"), Roaming, true}},
     {Config::Browser_UseCustomProxy, {QS("Browser/UseCustomProxy"), Roaming, false}},
     {Config::Browser_CustomProxyLocation, {QS("Browser/CustomProxyLocation"), Roaming, {}}},

src/core/Config.h

@@ -127,7 +127,6 @@ public:
         Browser_BestMatchOnly,
         Browser_UnlockDatabase,
         Browser_MatchUrlScheme,
-        Browser_SortByUsername,
         Browser_SupportBrowserProxy,
         Browser_UseCustomProxy,
         Browser_CustomProxyLocation,

tests/TestBrowser.cpp

@@ -448,46 +448,6 @@ void TestBrowser::testSubdomainsAndPaths()
     QCOMPARE(result.length(), 1);
 }
 
-void TestBrowser::testSortEntries()
-{
-    auto db = QSharedPointer<Database>::create();
-    auto* root = db->rootGroup();
-
-    QStringList urls = {"https://github.com/login_page",
-                        "https://github.com/login",
-                        "https://github.com/",
-                        "github.com/login",
-                        "http://github.com",
-                        "http://github.com/login",
-                        "github.com",
-                        "github.com/login?test=test",
-                        "https://github", // Invalid URL
-                        "github.com"};
-
-    auto entries = createEntries(urls, root);
-
-    browserSettings()->setBestMatchOnly(false);
-    browserSettings()->setSortByUsername(true);
-    auto result = m_browserService->sortEntries(entries, "https://github.com/login", "https://github.com/session");
-    QCOMPARE(result.size(), 10);
-    QCOMPARE(result[0]->username(), QString("User 1"));
-    QCOMPARE(result[0]->url(), urls[1]);
-    QCOMPARE(result[1]->username(), QString("User 3"));
-    QCOMPARE(result[1]->url(), urls[3]);
-    QCOMPARE(result[2]->username(), QString("User 7"));
-    QCOMPARE(result[2]->url(), urls[7]);
-    QCOMPARE(result[3]->username(), QString("User 0"));
-    QCOMPARE(result[3]->url(), urls[0]);
-
-    // Test with a perfect match. That should be first in the list.
-    result = m_browserService->sortEntries(entries, "https://github.com/login_page", "https://github.com/session");
-    QCOMPARE(result.size(), 10);
-    QCOMPARE(result[0]->username(), QString("User 0"));
-    QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
-    QCOMPARE(result[1]->username(), QString("User 1"));
-    QCOMPARE(result[1]->url(), QString("https://github.com/login"));
-}
-
 QList<Entry*> TestBrowser::createEntries(QStringList& urls, Group* root) const
 {
     QList<Entry*> entries;
@@ -598,8 +558,8 @@ void TestBrowser::testBestMatchingCredentials()
     result = m_browserService->searchEntries(db, siteUrl, siteUrl);
     sorted = m_browserService->sortEntries(result, siteUrl, siteUrl);
     QCOMPARE(sorted.size(), 2);
-    QCOMPARE(sorted[0]->url(), QString("https://subdomain.example.com/"));
+    QCOMPARE(sorted[0]->url(), QString("https://subdomain.example.com"));
-    QCOMPARE(sorted[1]->url(), QString("https://subdomain.example.com"));
+    QCOMPARE(sorted[1]->url(), QString("https://subdomain.example.com/"));
 
     // Entries with https://example.com should be still returned even if the site URL has a subdomain. Those have the
     // best match.

tests/TestBrowser.h

@@ -48,7 +48,6 @@ private slots:
     void testSearchEntriesWithAdditionalURLs();
     void testInvalidEntries();
     void testSubdomainsAndPaths();
-    void testSortEntries();
     void testValidURLs();
     void testBestMatchingCredentials();
     void testBestMatchingWithAdditionalURLs();