Merge pull request #168 from end2endzone/feature-issue164 #164 #155

Merge branch feature-issue164.

Author: end2endzone

CHANGES

@@ -1,14 +1,13 @@
 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().
+* Fixed issue #64: Fails to identify icon for HTML files.
 
 
 Changes for 0.9.0

src/core/Icon.cpp

@@ -50,25 +50,42 @@ namespace shellanything
   {
   }
 
-  const Icon& Icon::operator =(const Icon& icon)
+  const Icon& Icon::operator =(const Icon& other)
   {
-    if (this != &icon)
+    if (this != &other)
     {
-      mFileExtension = icon.mFileExtension;
-      mPath = icon.mPath;
-      mIndex = icon.mIndex;
+      mFileExtension = other.mFileExtension;
+      mPath = other.mPath;
+      mIndex = other.mIndex;
     }
     return (*this);
   }
 
+  bool Icon::operator ==(const Icon& other) const
+  {
+    if (this == &other)
+      return true;
+    if ((mFileExtension == other.mFileExtension) &&
+        (mPath == other.mPath) &&
+        (mIndex == other.mIndex))
+        return true;
+    return false;
+  }
+
   bool Icon::IsValid() const
   {
     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()
   {
@@ -89,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
@@ -150,6 +172,14 @@ namespace shellanything
     mIndex = index;
   }
 
+  Icon Icon::GetDefaultUnknownFileTypeIcon()
+  {
+    Icon tmp;
+    tmp.SetFileExtension("this_file_extension_is_not_registered_on_system");
+    tmp.ResolveFileExtensionIcon();
+    return tmp;
+  }
+
   std::string Icon::ToShortString() const
   {
     std::string str;

src/core/Icon.h

@@ -53,7 +53,20 @@ namespace shellanything
     /// <summary>
     /// Copy operator
     /// </summary>
-    const Icon& operator =(const Icon& icon);
+    const Icon& operator =(const Icon& other);
+
+    /// <summary>
+    /// Equal operator
+    /// </summary>
+    bool operator ==(const Icon& other) const;
+
+    /// <summary>
+    /// Not Equal operator
+    /// </summary>
+    inline bool operator !=(const Icon& other) const
+    {
+      return !((*this) == other);
+    }
 
     /// <summary>
     /// Returns true if the icon have a valid path and index.
@@ -96,6 +109,11 @@ namespace shellanything
     /// </summary>
     void SetIndex(const int& index);
 
+    /// <summary>
+    /// Get the system's default visual icon for unknown file type.
+    /// </summary>
+    static Icon GetDefaultUnknownFileTypeIcon();
+
     // IObject methods
     virtual std::string ToShortString() const;
     virtual void ToLongString(std::string& str, int indent) const;

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/shared/Win32Utils.cpp

@@ -1427,6 +1427,49 @@ namespace Win32Utils
     return hReturn;
   }
 
+  HBITMAP LoadIconFromFileAsBitmap32Bpp(const char* filename, int index)
+  {
+    HBITMAP hBitmap = INVALID_BITMAP_HANDLE;
+    HICON hIconSmall = NULL;
+
+    std::wstring icon_filename_wide = ra::unicode::Utf8ToUnicode(filename);
+
+    //check how many icons the file contains
+    UINT num_icon_in_file = ExtractIconExW(icon_filename_wide.c_str(), -1, NULL, NULL, 1);
+    if (num_icon_in_file == 0)
+    {
+      // ERROR: "File '" << filename << "' does not contains an icon.";
+    }
+    else
+    {
+      //the file contains 1 or more icons, try to load a small one
+      UINT num_icon_loaded = 0;
+      if (num_icon_in_file >= 1)
+        num_icon_loaded = ExtractIconExW(icon_filename_wide.c_str(), index, NULL, &hIconSmall, 1);
+      if (num_icon_in_file >= 1 && num_icon_loaded == 0)
+      {
+        // ERROR: "Failed to load icon index " << icon_index << " from file '" << icon_filename << "'.";
+      }
+      else
+      {
+        SIZE menu_icon_size = Win32Utils::GetIconSize(hIconSmall);
+        // ERROR: "Loaded icon " << icon_index << " from file '" << icon_filename << "' is " << menu_icon_size.cx << "x" << menu_icon_size.cy << ".";
+
+        //Convert the icon to a 32bpp bitmap with alpha channel (invisible background)
+        hBitmap = Win32Utils::CopyAsBitmap(hIconSmall);
+        if (hBitmap == INVALID_BITMAP_HANDLE)
+        {
+          // ERROR: "Icon " << icon_index << " from file '" << icon_filename << "' has failed to convert to bitmap.";
+        }
+
+        if (hIconSmall != NULL)
+          DestroyIcon(hIconSmall);
+      }
+    }
+
+    return hBitmap;
+  }
+
   BOOL IsFullyTransparent(HBITMAP hBitmap)
   {
     SIZE bitmap_size = GetBitmapSize(hBitmap);

src/shared/Win32Utils.h

@@ -30,6 +30,8 @@
 
 namespace Win32Utils
 {
+  static const HBITMAP INVALID_BITMAP_HANDLE = NULL;
+
   void GetWindowsVersion(int& major, int& minor);
   std::string GetWindowsProductName();
   bool EnableMonitorDpiAwareness();
@@ -59,6 +61,7 @@ namespace Win32Utils
   bool SaveAs32BppBitmapV3File(const char* path, HBITMAP hBitmap);
   bool SaveBitmapFile(const char* path, HBITMAP hBitmap);
   HBITMAP LoadBitmapFromFile(const char* path);
+  HBITMAP LoadIconFromFileAsBitmap32Bpp(const char* filename, int index);
   BOOL IsFullyTransparent(HBITMAP hBitmap);
   BOOL IsFullyTransparent(const std::string& buffer);
   std::string GetMenuItemDetails(HMENU hMenu, UINT pos);

src/tests/TestIcon.cpp

@@ -65,6 +65,39 @@ namespace shellanything
     {
     }
     //--------------------------------------------------------------------------------------------------
+    TEST_F(TestIcon, testOperatorEquals)
+    {
+      Icon iconA;
+      Icon iconB;
+
+      #define ASSERT_ICON_EQUALS(iconA, iconB) ASSERT_TRUE(iconA == iconB); ASSERT_FALSE(iconA != iconB);
+      #define ASSERT_ICON_NOT_EQUALS(iconA, iconB) ASSERT_TRUE(iconA != iconB); ASSERT_FALSE(iconA == iconB);
+
+      // assert equals
+      ASSERT_ICON_EQUALS(iconA, iconB);
+
+      iconA.SetFileExtension("txt");
+      ASSERT_ICON_NOT_EQUALS(iconA, iconB);
+
+      iconA.SetFileExtension("exe");
+      iconB.SetFileExtension("exe");
+      ASSERT_ICON_EQUALS(iconA, iconB);
+
+      iconA.SetPath("foo.dll");
+      ASSERT_ICON_NOT_EQUALS(iconA, iconB);
+
+      iconA.SetPath("bar.dll");
+      iconB.SetPath("bar.dll");
+      ASSERT_ICON_EQUALS(iconA, iconB);
+
+      iconA.SetIndex(11);
+      ASSERT_ICON_NOT_EQUALS(iconA, iconB);
+
+      iconA.SetIndex(99);
+      iconB.SetIndex(99);
+      ASSERT_ICON_EQUALS(iconA, iconB);
+    }
+    //--------------------------------------------------------------------------------------------------
     TEST_F(TestIcon, testValidIcon)
     {
       // default ctor
@@ -100,11 +133,11 @@ namespace shellanything
         Icon icon;
         icon.SetPath("path/to/a/file.ico");
         icon.SetIndex(-99);
-        ASSERT_FALSE(icon.IsValid());
+        ASSERT_TRUE(icon.IsValid());
       }
     }
     //--------------------------------------------------------------------------------------------------
-    TEST_F(TestIcon, testResolveFileExtensionIcon)
+    TEST_F(TestIcon, testResolveFileExtensionIconTxt)
     {
       Icon icon;
       icon.SetFileExtension("txt");
@@ -115,6 +148,36 @@ namespace shellanything
       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, 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();
+
+      ASSERT_TRUE(icon.IsValid());
+      ASSERT_TRUE(icon.GetFileExtension().empty());
+
+      ASSERT_FALSE(icon.GetPath().empty());
+      ASSERT_NE(Icon::INVALID_ICON_INDEX, icon.GetIndex());
     }
     //--------------------------------------------------------------------------------------------------
     TEST_F(TestIcon, testResolveMultipleFileExtensionIcon) // issue #98

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