[REGEDIT] Fix ListView selection and finding (#5150)

We will check the data size correctly, instead of 3 NUL byte appending hack. Add bSelectNone parameter to UpdateAddress and RefreshListView functions. If bSelectNone is TRUE, then select nothing of ListView. Fix item selection of ListView. Rename CompareData helper function as MatchData and improve it. Improve the search algorithm. If the item selection of ListView changed, scroll down to the item. Follow up to #5146. CORE-15986, CORE-18230

diff --git a/base/applications/regedit/childwnd.c b/base/applications/regedit/childwnd.c
index 19c4cea..7910dce 100644 (file)
--- a/base/applications/regedit/childwnd.c
+++ b/base/applications/regedit/childwnd.c
@@ -229,7 +229,7 @@ LRESULT CALLBACK AddressBarProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lPar
 }
 
 VOID
-UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath)
+UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone)
 {
     LPCWSTR keyPath, rootName;
     LPWSTR fullPath;
@@ -251,7 +251,7 @@ UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath)
 
     if (keyPath)
     {
-        RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath, bSelectNone);
         rootName = get_root_key_name(hRootKey);
         cbFullPath = (wcslen(rootName) + 1 + wcslen(keyPath) + 1) * sizeof(WCHAR);
         fullPath = malloc(cbFullPath);

diff --git a/base/applications/regedit/find.c b/base/applications/regedit/find.c
index c970a53..444f4a2 100644 (file)
--- a/base/applications/regedit/find.c
+++ b/base/applications/regedit/find.c
@@ -87,41 +87,32 @@ static BOOL CompareName(LPCWSTR pszName1, LPCWSTR pszName2)
     }
 }
 
-static BOOL
-CompareData(
-    DWORD   dwType,
-    LPCWSTR psz1,
-    LPCWSTR psz2)
+/* Do not assume that pch1 is terminated with UNICODE_NULL */
+static BOOL MatchString(LPCWCH pch1, INT cch1, LPCWCH pch2, INT cch2)
 {
-    INT i, cch1 = wcslen(psz1), cch2 = wcslen(psz2);
-    if (dwType == REG_SZ || dwType == REG_EXPAND_SZ)
-    {
-        if (s_dwFlags & RSF_WHOLESTRING)
-        {
-            if (s_dwFlags & RSF_MATCHCASE)
-                return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0,
-                                          psz1, cch1, psz2, cch2);
-            else
-                return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT,
-                                          NORM_IGNORECASE, psz1, cch1, psz2, cch2);
-        }
+    INT i;
+    DWORD dwNorm = ((s_dwFlags & RSF_MATCHCASE) ? NORM_IGNORECASE : 0);
 
-        for(i = 0; i <= cch1 - cch2; i++)
-        {
-            if (s_dwFlags & RSF_MATCHCASE)
-            {
-                if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0,
-                                       psz1 + i, cch2, psz2, cch2))
-                    return TRUE;
-            }
-            else
-            {
-                if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT,
-                                       NORM_IGNORECASE, psz1 + i, cch2, psz2, cch2))
-                    return TRUE;
-            }
-        }
+    if (s_dwFlags & RSF_WHOLESTRING)
+        return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1, cch1, pch2, cch2);
+
+    if (cch1 < cch2)
+        return FALSE;
+
+    for (i = 0; i <= cch1 - cch2; i++)
+    {
+        if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1 + i, cch2, pch2, cch2))
+            return TRUE;
     }
+
+    return FALSE;
+}
+
+static BOOL MatchData(DWORD dwType, LPCVOID pv1, DWORD cb1)
+{
+    if (dwType == REG_SZ || dwType == REG_EXPAND_SZ || dwType == REG_MULTI_SZ)
+        return MatchString(pv1, (INT)(cb1 / sizeof(WCHAR)), s_szFindWhat, lstrlenW(s_szFindWhat));
+
     return FALSE;
 }
 
@@ -143,7 +134,7 @@ BOOL RegFindRecurse(
     LONG lResult;
     WCHAR szSubKey[MAX_PATH];
     DWORD i, c, cb, type;
-    BOOL fPast = FALSE;
+    BOOL fPast;
     LPWSTR *ppszNames = NULL;
     LPBYTE pb = NULL;
 
@@ -169,6 +160,7 @@ BOOL RegFindRecurse(
         goto err;
     ZeroMemory(ppszNames, c * sizeof(LPWSTR));
 
+    /* Retrieve the value names associated with the current key */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -192,10 +184,11 @@ BOOL RegFindRecurse(
 
     qsort(ppszNames, c, sizeof(LPWSTR), compare);
 
-    if (pszValueName == NULL)
-        pszValueName = ppszNames[0];
+    /* If pszValueName is NULL, the function will search for all values within the key */
+    fPast = (pszValueName == NULL);
 
-    for(i = 0; i < c; i++)
+    /* Search within the values */
+    for (i = 0; i < c; i++)
     {
         if (DoEvents())
             goto err;
@@ -212,10 +205,7 @@ BOOL RegFindRecurse(
                 CompareName(ppszNames[i], s_szFindWhat))
         {
             *ppszFoundSubKey = _wcsdup(szSubKey);
-            if (ppszNames[i][0] == 0)
-                *ppszFoundValueName = NULL;
-            else
-                *ppszFoundValueName = _wcsdup(ppszNames[i]);
+            *ppszFoundValueName = _wcsdup(ppszNames[i]);
             goto success;
         }
 
@@ -223,7 +213,7 @@ BOOL RegFindRecurse(
                                   NULL, &cb);
         if (lResult != ERROR_SUCCESS)
             goto err;
-        pb = malloc(cb + 3); /* To avoid buffer overrun, append 3 NULs */
+        pb = malloc(cb);
         if (pb == NULL)
             goto err;
         lResult = RegQueryValueExW(hSubKey, ppszNames[i], NULL, &type,
@@ -231,19 +221,10 @@ BOOL RegFindRecurse(
         if (lResult != ERROR_SUCCESS)
             goto err;
 
-        /* To avoid buffer overrun, append 3 NUL bytes.
-           NOTE: cb can be an odd number although UNICODE_NULL is two bytes.
-           Two bytes at odd position is not enough to avoid buffer overrun. */
-        pb[cb] = pb[cb + 1] = pb[cb + 2] = 0;
-
-        if ((s_dwFlags & RSF_LOOKATDATA) &&
-                CompareData(type, (LPWSTR) pb, s_szFindWhat))
+        if ((s_dwFlags & RSF_LOOKATDATA) && MatchData(type, pb, cb))
         {
             *ppszFoundSubKey = _wcsdup(szSubKey);
-            if (ppszNames[i][0] == 0)
-                *ppszFoundValueName = NULL;
-            else
-                *ppszFoundValueName = _wcsdup(ppszNames[i]);
+            *ppszFoundValueName = _wcsdup(ppszNames[i]);
             goto success;
         }
         free(pb);
@@ -258,6 +239,7 @@ BOOL RegFindRecurse(
     }
     ppszNames = NULL;
 
+    /* Retrieve the number of sub-keys */
     lResult = RegQueryInfoKeyW(hSubKey, NULL, NULL, NULL, &c, NULL, NULL,
                               NULL, NULL, NULL, NULL, NULL);
     if (lResult != ERROR_SUCCESS)
@@ -267,6 +249,7 @@ BOOL RegFindRecurse(
         goto err;
     ZeroMemory(ppszNames, c * sizeof(LPWSTR));
 
+    /* Retrieve the names of the sub-keys */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -290,6 +273,7 @@ BOOL RegFindRecurse(
 
     qsort(ppszNames, c, sizeof(LPWSTR), compare);
 
+    /* Search within the sub-keys */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -315,6 +299,7 @@ BOOL RegFindRecurse(
             goto success;
         }
 
+        /* Search within the value entries of the sub-key */
         if (RegFindRecurse(hSubKey, ppszNames[i], NULL, ppszFoundSubKey,
                            ppszFoundValueName))
         {
@@ -371,7 +356,7 @@ BOOL RegFindWalk(
     WCHAR szKeyName[MAX_PATH];
     WCHAR szSubKey[MAX_PATH];
     LPWSTR pch;
-    BOOL fPast;
+    BOOL fPast, fKeyMatched;
     LPWSTR *ppszNames = NULL;
 
     hBaseKey = *phKey;
@@ -470,7 +455,8 @@ BOOL RegFindWalk(
                 goto success;
             }
 
-            if (RegFindRecurse(hSubKey, ppszNames[i], NULL,
+            fKeyMatched = (_wcsicmp(ppszNames[i], szKeyName) == 0);
+            if (RegFindRecurse(hSubKey, ppszNames[i], (fKeyMatched ? pszValueName : NULL),
                                ppszFoundSubKey, ppszFoundValueName))
             {
                 LPWSTR psz = *ppszFoundSubKey;

diff --git a/base/applications/regedit/framewnd.c b/base/applications/regedit/framewnd.c
index e3d695a..f886e0c 100644 (file)
--- a/base/applications/regedit/framewnd.c
+++ b/base/applications/regedit/framewnd.c
@@ -383,7 +383,7 @@ static BOOL LoadHive(HWND hWnd)
                 /* refresh tree and list views */
                 RefreshTreeView(g_pChildWnd->hTreeWnd);
                 pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey);
-                RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+                RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
             }
             else
             {
@@ -421,7 +421,7 @@ static BOOL UnloadHive(HWND hWnd)
         /* refresh tree and list views */
         RefreshTreeView(g_pChildWnd->hTreeWnd);
         pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey);
-        RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
     }
     else
     {
@@ -518,7 +518,7 @@ static BOOL ImportRegistryFile(HWND hWnd)
     /* refresh tree and list views */
     RefreshTreeView(g_pChildWnd->hTreeWnd);
     pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot);
-    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath);
+    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath, TRUE);
 
     return bRet;
 }
@@ -877,7 +877,7 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType)
         return FALSE;
     }
 
-    RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+    RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
 
     /* locate the newly added value, and get ready to rename it */
     memset(&lvfi, 0, sizeof(lvfi));
@@ -885,7 +885,12 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType)
     lvfi.psz = szNewValue;
     iIndex = ListView_FindItem(g_pChildWnd->hListWnd, -1, &lvfi);
     if (iIndex >= 0)
+    {
+        ListView_SetItemState(g_pChildWnd->hListWnd, iIndex,
+                              LVIS_SELECTED | LVIS_FOCUSED, LVIS_SELECTED | LVIS_FOCUSED);
+        ListView_EnsureVisible(g_pChildWnd->hListWnd, iIndex, FALSE);
         (void)ListView_EditLabel(g_pChildWnd->hListWnd, iIndex);
+    }
 
     return TRUE;
 }
@@ -1128,11 +1133,11 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
     {
     case ID_EDIT_MODIFY:
         if (valueName && ModifyValue(hWnd, hKey, valueName, FALSE))
-            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
         break;
     case ID_EDIT_MODIFY_BIN:
         if (valueName && ModifyValue(hWnd, hKey, valueName, TRUE))
-            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
         break;
     case ID_EDIT_RENAME:
         if (GetFocus() == g_pChildWnd->hListWnd)
@@ -1180,7 +1185,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
                         item = ni;
                     }
 
-                    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+                    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
                     if(errs > 0)
                     {
                         LoadStringW(hInst, IDS_ERR_DELVAL_CAPTION, caption, ARRAY_SIZE(caption));
@@ -1243,7 +1248,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
     case ID_VIEW_REFRESH:
         RefreshTreeView(g_pChildWnd->hTreeWnd);
         keyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot);
-        RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, TRUE);
         break;
         /*case ID_OPTIONS_TOOLBAR:*/
         /*     toggle_child(hWnd, LOWORD(wParam), hToolBar);*/

diff --git a/base/applications/regedit/listview.c b/base/applications/regedit/listview.c
index ce0d540..346ed78 100644 (file)
--- a/base/applications/regedit/listview.c
+++ b/base/applications/regedit/listview.c
@@ -91,7 +91,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName)
     {
         ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED);
     }
-    if (pszValueName == NULL)
+    if (pszValueName == NULL || pszValueName[0] == 0)
         i = 0;
     else
     {
@@ -101,6 +101,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName)
     }
     ListView_SetItemState(hwndLV, i, LVIS_FOCUSED | LVIS_SELECTED,
                           LVIS_FOCUSED | LVIS_SELECTED);
+    ListView_EnsureVisible(hwndLV, i, FALSE);
     iListViewSelect = i;
 }
 
@@ -668,7 +669,7 @@ void DestroyListView(HWND hwndLV)
 
 }
 
-BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath)
+BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone)
 {
     DWORD max_sub_key_len;
     DWORD max_val_name_len;
@@ -738,6 +739,9 @@ BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath)
     {
         ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED);
     }
+
+    if (bSelectNone)
+        iListViewSelect = -1;
     ListView_SetItemState(hwndLV, iListViewSelect,
                           LVIS_FOCUSED | LVIS_SELECTED,
                           LVIS_FOCUSED | LVIS_SELECTED);

diff --git a/base/applications/regedit/main.h b/base/applications/regedit/main.h
index 7c8b864..bb257e7 100644 (file)
--- a/base/applications/regedit/main.h
+++ b/base/applications/regedit/main.h
@@ -97,7 +97,7 @@ void ShowAboutBox(HWND hWnd);
 LRESULT CALLBACK ChildWndProc(HWND, UINT, WPARAM, LPARAM);
 void ResizeWnd(int cx, int cy);
 LPCWSTR get_root_key_name(HKEY hRootKey);
-VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath);
+VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone);
 
 /* edit.c */
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCWSTR valueName, BOOL EditBin);
@@ -125,7 +125,7 @@ BOOL ExportRegistryFile(HWND hWnd);
 
 /* listview.c */
 HWND CreateListView(HWND hwndParent, HMENU id, INT cx);
-BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath);
+BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone);
 WCHAR *GetValueName(HWND hwndLV, int iStartAt);
 BOOL ListWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result);
 BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result);

diff --git a/base/applications/regedit/treeview.c b/base/applications/regedit/treeview.c
index ee26a04..2822243 100644 (file)
--- a/base/applications/regedit/treeview.c
+++ b/base/applications/regedit/treeview.c
@@ -643,7 +643,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result)
             /* Get the parent of the current item */
             HTREEITEM hParentItem = TreeView_GetParent(g_pChildWnd->hTreeWnd, pnmtv->itemNew.hItem);
 
-            UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL);
+            UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL, TRUE);
 
             /* Disable the Permissions menu item for 'My Computer' */
             EnableMenuItem(hMenuFrame, ID_EDIT_PERMISSIONS, MF_BYCOMMAND | (hParentItem ? MF_ENABLED : MF_GRAYED));
@@ -722,7 +722,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result)
                         lResult = FALSE;
                     }
                     else
-                        UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer);
+                        UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer, FALSE);
                 }
                 *Result = lResult;
             }