Modified Icon and Win32Registry to properly support icons with negative index. The api ExtractIconExW() supports negative index.

This effectively reverts changes made for #155.

Author: end2endzone

CHANGES

@@ -1,11 +1,9 @@
 Changes for 0.10.0
 
-* Deprecated support for icons with negative resource id.
 * Fixed issue #6  : (twice) Right-click on a directory with Windows Explorer in the left panel shows the menus twice.
 * Fixed issue #31 : (twice) Error in logs for CContextMenu::GetCommandString()
 * Fixed issue #109: Implement default and verbose logging.
 * Fixed issue #150: ico icon (that do not specifically force index=0) are not working.
-* Fixed issue #155: Drop support for loading icons from a resource id (icons with a negative index smaller than -1).
 * Fixed issue #157: Compilation fails on Github Action: `fatal error C1083: Cannot open include file: 'atlbase.h': No such file or directory`.
 * Fixed issue #158: Compilation fails on Github Action: `CPack error : Problem running WiX.`.
 * Fixed issue #159: Unit test fails on Github Actions: TestPlugins.testServices().

src/core/Icon.cpp

@@ -76,10 +76,16 @@ namespace shellanything
   {
     if (!mFileExtension.empty())
       return true;
-    if (!mPath.empty() && mIndex >= 0) // not a resource id. See Issue #17, #150, #155
+
+    //See issue #17, 155, 164.
+    //An icon with a negative index is valid from the registry.
+    //Only the special case index = -1 should be considered invalid (Issue #17).
+    //And ShellAnything accept positive (index) and negative index (resource id). (Issue #155, Issue #164).
+    if (!mPath.empty() && mIndex != INVALID_ICON_INDEX)
       return true;
+
     return false;
-  }
+}
 
   void Icon::ResolveFileExtensionIcon()
   {
@@ -100,7 +106,12 @@ namespace shellanything
 
       //try to find the path to the icon module for the given file extension.
       Win32Registry::REGISTRY_ICON resolved_icon = Win32Registry::GetFileTypeIcon(file_extension.c_str());
-      if (!resolved_icon.path.empty() && resolved_icon.index >= 0) // See Issue #17 #155. Do not accept icons which are resource ids.
+
+      //An icon with a negative index is valid from the registry.
+      //Only the special case index = -1 should be considered invalid (Issue #17).
+      //And ShellAnything accept positive (index) and negative index (resource id). (Issue #155, Issue #164).
+      //See issue #17, 155, 164.
+      if (Win32Registry::IsValid(resolved_icon))
       {
         //found the icon for the file extension
         //replace this menu's icon with the new information

src/shared/Win32Registry.cpp

@@ -684,8 +684,13 @@ namespace Win32Registry
   {
     if (icon.path.empty())
       return false;
-    if (icon.index == INVALID_ICON_INDEX) //See issue #17
+
+    //See issue #17, 155, 164.
+    //An icon with a negative index is valid from the registry.
+    //Only the special case index = -1 should be considered invalid (Issue #17).
+    if (icon.index == INVALID_ICON_INDEX)
       return false;
+
     if (IsIconEquals(icon, NULL_ICON))
       return false;
     return true;

src/tests/TestIcon.cpp

@@ -133,7 +133,7 @@ namespace shellanything
         Icon icon;
         icon.SetPath("path/to/a/file.ico");
         icon.SetIndex(-99);
-        ASSERT_FALSE(icon.IsValid());
+        ASSERT_TRUE(icon.IsValid());
       }
     }
     //--------------------------------------------------------------------------------------------------
@@ -153,6 +153,22 @@ namespace shellanything
       ASSERT_TRUE(icon != Icon::GetDefaultUnknownFileTypeIcon());
     }
     //--------------------------------------------------------------------------------------------------
+    TEST_F(TestIcon, testResolveFileExtensionIconHtml)
+    {
+      Icon icon;
+      icon.SetFileExtension("html");
+
+      //act
+      icon.ResolveFileExtensionIcon();
+
+      ASSERT_TRUE(icon.GetFileExtension().empty());
+      ASSERT_FALSE(icon.GetPath().empty());
+      ASSERT_NE(Icon::INVALID_ICON_INDEX, icon.GetIndex());
+
+      // assert we did not resolve to default unknown icon
+      ASSERT_TRUE(icon != Icon::GetDefaultUnknownFileTypeIcon());
+    }
+    //--------------------------------------------------------------------------------------------------
     TEST_F(TestIcon, testGetDefaultUnknownFileTypeIcon)
     {
       Icon icon = Icon::GetDefaultUnknownFileTypeIcon();

src/tests/TestWin32Utils.cpp

@@ -28,6 +28,7 @@
 #pragma warning( pop )
 
 #include "TestWin32Utils.h"
+#include "Icon.h"
 #include "Win32Utils.h"
 #include "ArgumentsHandler.h"
 
@@ -652,6 +653,68 @@ namespace shellanything
       }
     }
     //--------------------------------------------------------------------------------------------------
+    TEST_F(TestWin32Utils, testNegativeIconIndexIssue155) // Issue #155
+    {
+      Icon icon;
+
+      // On my system, with current implementation,
+      // Icon::ResolveFileExtensionIcon() resolves HTML file extension to the following:
+      icon.SetPath("C:\\Program Files\\Internet Explorer\\iexplore.exe");
+      icon.SetIndex(-17);
+
+      // act, load an icon with negative index
+      HBITMAP hBitmap = Win32Utils::LoadIconFromFileAsBitmap32Bpp(icon.GetPath().c_str(), icon.GetIndex());
+      ASSERT_NE(hBitmap, Win32Utils::INVALID_BITMAP_HANDLE);
+
+      // build output files
+      std::string test_dir = ra::filesystem::GetTemporaryDirectory();
+      ASSERT_TRUE(ra::filesystem::CreateDirectory(test_dir.c_str()));
+      std::string bitmap_filename = ra::testing::GetTestQualifiedName() + ".bmp";
+      std::string output_path = test_dir + "\\" + bitmap_filename;
+
+      // remove data from previous runs
+      ra::filesystem::DeleteFile(output_path.c_str());
+
+      // Save as *.bmp
+      bool saved = Win32Utils::SaveBitmapFile(output_path.c_str(), hBitmap);
+      ASSERT_TRUE(saved) << "Failed to save bitmap to file: " << output_path;
+
+      // Cleanup
+      DeleteObject(hBitmap);
+    }
+    //--------------------------------------------------------------------------------------------------
+    TEST_F(TestWin32Utils, testNegativeIconIndexIssue164) // Issue #164
+    {
+      Icon icon;
+
+      // On my system, Icon::ResolveFileExtensionIcon() resolves 'txt' file extension to a negative icon index
+      icon.SetFileExtension("txt");
+      icon.ResolveFileExtensionIcon();
+
+      ASSERT_TRUE(icon.IsValid());
+      ASSERT_TRUE(icon.GetIndex() < 1);
+
+      // act, load an icon with negative index
+      HBITMAP hBitmap = Win32Utils::LoadIconFromFileAsBitmap32Bpp(icon.GetPath().c_str(), icon.GetIndex());
+      ASSERT_NE(hBitmap, Win32Utils::INVALID_BITMAP_HANDLE);
+
+      // build output files
+      std::string test_dir = ra::filesystem::GetTemporaryDirectory();
+      ASSERT_TRUE(ra::filesystem::CreateDirectory(test_dir.c_str()));
+      std::string bitmap_filename = ra::testing::GetTestQualifiedName() + ".bmp";
+      std::string output_path = test_dir + "\\" + bitmap_filename;
+
+      // remove data from previous runs
+      ra::filesystem::DeleteFile(output_path.c_str());
+
+      // Save as *.bmp
+      bool saved = Win32Utils::SaveBitmapFile(output_path.c_str(), hBitmap);
+      ASSERT_TRUE(saved) << "Failed to save bitmap to file: " << output_path;
+
+      // Cleanup
+      DeleteObject(hBitmap);
+    }
+    //--------------------------------------------------------------------------------------------------
 
   } //namespace test
 } //namespace shellanything