[PATCH 0/1] MR8526: hhctrl.ocx: Selection of treeview items in the content tab should reflect web browser page.

[Jacob Czekalla (＠JacobCzekalla)]

Currently the treeview items do not change selection when back or forward buttons are clicked. The DoSync function also needed to be changed because the paths to the htm files in the .chm don't line up with the path to the .chm, especially if the .chm is not in the same path as hh.exe.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526

[PATCH 1/1] hhctrl.ocx: Selection of treeview items in the content tab should reflect web browser page.

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

Currently the treeview items do not change selection when back or forward buttons are clicked.
---
 dlls/hhctrl.ocx/help.c   | 145 ++++++++++++++++++++++++++++++++++++---
 dlls/hhctrl.ocx/hhctrl.h |  13 ++++
 2 files changed, 147 insertions(+), 11 deletions(-)

diff --git a/dlls/hhctrl.ocx/help.c b/dlls/hhctrl.ocx/help.c
index 413659c7097..060fcfec336 100644
--- a/dlls/hhctrl.ocx/help.c
+++ b/dlls/hhctrl.ocx/help.c
@@ -25,6 +25,7 @@
 #include "wingdi.h"
 #include "commctrl.h"
 #include "wininet.h"
+#include "exdispid.h"
 
 #include "wine/debug.h"
 
@@ -258,12 +259,21 @@ BOOL NavigateToChm(HHInfo *info, LPCWSTR file, LPCWSTR index)
     return SUCCEEDED(navigate_url(info, buf));
 }
 
-static void DoSync(HHInfo *info)
+static BOOL is_chm(WCHAR *url)
 {
-    WCHAR buf[INTERNET_MAX_URL_LENGTH];
+    const WCHAR *prefix = L"mk:@MSITStore:";
+    return !wcsncmp(url, prefix, wcslen(prefix));
+}
+
+static void DoSyncContent(HHInfo *info)
+{
+    const WCHAR *index;
     HRESULT hres;
     BSTR url;
 
+    if (info->current_tab != TAB_CONTENTS)
+        return;
+
     hres = IWebBrowser2_get_LocationURL(info->web_browser->web_browser, &url);
 
     if (FAILED(hres))
@@ -272,24 +282,104 @@ static void DoSync(HHInfo *info)
         return;
     }
 
-    /* If we're not currently viewing a page in the active .chm file, abort */
-    if ((!AppendFullPathURL(info->WinType.pszFile, buf, NULL)) || (lstrlenW(buf) > lstrlenW(url)))
+    /* If we're not currently viewing a page in a .chm file, abort */
+    if (!is_chm(url))
     {
         SysFreeString(url);
         return;
     }
 
-    if (lstrcmpiW(buf, url) > 0)
+    index = wcsstr(url, L"::/");
+
+    if (index)
+        ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */
+
+    SysFreeString(url);
+}
+
+static HRESULT WINAPI WebBrowserEvents2_QueryInterface(IDispatch *iface, REFIID riid, void **v)
+{
+    *v = NULL;
+
+    if(IsEqualGUID(&DIID_DWebBrowserEvents2, riid) || IsEqualGUID(&IID_IDispatch, riid)) {
+        *v = iface;
+        return S_OK;
+    }
+
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI Dispatch_AddRef(IDispatch *iface)
+{
+    WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
+    return InterlockedIncrement(&impl->ref);
+}
+static ULONG WINAPI Dispatch_Release(IDispatch *iface)
+{
+    WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
+    ULONG ref = InterlockedDecrement(&impl->ref);
+
+    if (!ref)
+        free(impl);
+    return ref;
+}
+
+static HRESULT WINAPI WebBrowserEvents2_Invoke(IDispatch *iface, DISPID dispIdMember, REFIID riid,
+        LCID lcid, WORD wFlags, DISPPARAMS *pDispParams, VARIANT *pVarResult,
+        EXCEPINFO *pExcepInfo, UINT *puArgErr)
+{
+    if (dispIdMember == DISPID_NAVIGATECOMPLETE2)
     {
-        const WCHAR *index;
+        WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
+        DoSyncContent((HHInfo *)impl->info);
+    }
+    return S_OK;
+}
 
-        index = wcsstr(url, L"::/");
+static IDispatchVtbl WebBrowserEvents2Vtbl =
+{
+    WebBrowserEvents2_QueryInterface,
+    Dispatch_AddRef,
+    Dispatch_Release,
+    NULL,
+    NULL,
+    NULL,
+    WebBrowserEvents2_Invoke
+};
 
-        if (index)
-            ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */
+static void hook_nav_complete(HHInfo *info, BOOL init)
+{
+    IConnectionPointContainer *container;
+    IConnectionPoint *point;
+    HRESULT hres;
+
+    static DWORD dw = 100;
+
+    hres = IWebBrowser2_QueryInterface(info->web_browser->web_browser, &IID_IConnectionPointContainer, (void **)&container);
+    if (FAILED(hres))
+        return;
+
+    hres = IConnectionPointContainer_FindConnectionPoint(container, &DIID_DWebBrowserEvents2, &point);
+    IConnectionPointContainer_Release(container);
+    if (FAILED(hres))
+        return;
+
+    if (init)
+    {
+        info->navigate_sink = malloc(sizeof(*(info->navigate_sink)));
+        info->navigate_sink->iface.lpVtbl = &WebBrowserEvents2Vtbl;
+        info->navigate_sink->lpVtbl = &WebBrowserEvents2Vtbl;
+        info->navigate_sink->info = (struct HHInfo *)info;
+        info->navigate_sink->ref = 1;
+        IConnectionPoint_Advise(point, (IUnknown *)info->navigate_sink, &dw);
+    }
+    else
+    {
+        IConnectionPoint_Unadvise(point, dw);
+        IDispatch_Release((IDispatch *)info->navigate_sink);
     }
 
-    SysFreeString(url);
+    IConnectionPoint_Release(point);
 }
 
 /* Size Bar */
@@ -577,9 +667,36 @@ static LRESULT OnTabChange(HWND hwnd)
     if(info->tabs[info->current_tab].hwnd)
         ShowWindow(info->tabs[info->current_tab].hwnd, SW_SHOW);
 
+    if (info->current_tab == TAB_CONTENTS)
+        DoSyncContent(info);
+
     return 0;
 }
 
+static BOOL is_current_page(HHInfo *info, const WCHAR *local)
+{
+    WCHAR* url;
+    WCHAR* current_page;
+    HRESULT res;
+
+    res = IWebBrowser2_get_LocationURL(info->web_browser->web_browser, &url);
+    if (FAILED(res))
+        return FALSE;
+
+    if (is_chm(url))
+    {
+        current_page = wcsstr(url, L"::/");
+        if (current_page && !lstrcmpW(local, current_page + 3))
+            return TRUE;
+    }else
+    {
+        if (!lstrcmpW(local, url))
+            return TRUE;
+    }
+
+    return FALSE;
+}
+
 static LRESULT OnTopicChange(HHInfo *info, void *user_data)
 {
     LPCWSTR chmfile = NULL, name = NULL, local = NULL;
@@ -596,6 +713,9 @@ static LRESULT OnTopicChange(HHInfo *info, void *user_data)
         citer = (ContentItem *) user_data;
         name = citer->name;
         local = citer->local;
+        if (is_current_page(info, local))
+            return 0;
+
         while(citer) {
             if(citer->merge.chm_file) {
                 chmfile = citer->merge.chm_file;
@@ -880,7 +1000,7 @@ static void TB_OnClick(HWND hWnd, DWORD dwID)
             ExpandContract(info);
             break;
         case IDTB_SYNC:
-            DoSync(info);
+            DoSyncContent(info);
             break;
         case IDTB_OPTIONS:
             DisplayPopupMenu(info);
@@ -1158,6 +1278,8 @@ static BOOL HH_AddHTMLPane(HHInfo *pHHInfo)
     if (!InitWebBrowser(pHHInfo, hWnd))
         return FALSE;
 
+    hook_nav_complete(pHHInfo, TRUE);
+
     /* store the pointer to the HH info struct */
     SetWindowLongPtrW(hWnd, 0, (LONG_PTR)pHHInfo);
 
@@ -1785,6 +1907,7 @@ void ReleaseHelpViewer(HHInfo *info)
     if (info->pCHMInfo)
         CloseCHM(info->pCHMInfo);
 
+    hook_nav_complete(info, FALSE);
     ReleaseWebBrowser(info);
     ReleaseContent(info);
     ReleaseIndex(info);
diff --git a/dlls/hhctrl.ocx/hhctrl.h b/dlls/hhctrl.ocx/hhctrl.h
index ebec0eebf30..42fe6616d50 100644
--- a/dlls/hhctrl.ocx/hhctrl.h
+++ b/dlls/hhctrl.ocx/hhctrl.h
@@ -181,6 +181,17 @@ typedef struct {
     HWND hwndWindow;
 } WebBrowserContainer;
 
+struct HHInfo;
+
+typedef struct WebBrowserEvents2Impl
+{
+    IDispatch iface;
+    IDispatchVtbl *lpVtbl;
+
+    struct HHInfo *info;
+    LONG ref;
+} WebBrowserEvents2Impl;
+
 typedef struct {
     WebBrowserContainer *web_browser;
 
@@ -203,6 +214,8 @@ typedef struct {
     HHTab tabs[TAB_FAVORITES+1];
     int viewer_initialized;
     DWORD current_tab;
+
+    WebBrowserEvents2Impl *navigate_sink;
 } HHInfo;
 
 BOOL InitWebBrowser(HHInfo*,HWND);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8526

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +{
> +    if (dispIdMember == DISPID_NAVIGATECOMPLETE2)
>      {
> -        const WCHAR *index;
> +        WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
> +        DoSyncContent((HHInfo *)impl->info);
> +    }
> +    return S_OK;
> +}
>
> -        index = wcsstr(url, L"::/");
> +static IDispatchVtbl WebBrowserEvents2Vtbl =
> +{
> +    WebBrowserEvents2_QueryInterface,
> +    Dispatch_AddRef,
> +    Dispatch_Release,
> It's more readable to have matching prefix, so Dispatch_* used for the same object is not ideal.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109138

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +
> +static HRESULT WINAPI WebBrowserEvents2_QueryInterface(IDispatch *iface, REFIID riid, void **v)
> +{
> +    *v = NULL;
> +
> +    if(IsEqualGUID(&DIID_DWebBrowserEvents2, riid) || IsEqualGUID(&IID_IDispatch, riid)) {
> +        *v = iface;
> +        return S_OK;
> +    }
> +
> +    return E_NOINTERFACE;
> +}
> +
> +static ULONG WINAPI Dispatch_AddRef(IDispatch *iface)
> +{
> +    WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
> Please use impl_from_* helpers for this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109137

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> -        const WCHAR *index;
> +        WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
> +        DoSyncContent((HHInfo *)impl->info);
> +    }
> +    return S_OK;
> +}
>
> -        index = wcsstr(url, L"::/");
> +static IDispatchVtbl WebBrowserEvents2Vtbl =
> +{
> +    WebBrowserEvents2_QueryInterface,
> +    Dispatch_AddRef,
> +    Dispatch_Release,
> +    NULL,
> +    NULL,
> +    NULL,
> This needs to be filled out with stubs, even if you think it's not used.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109139

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +    index = wcsstr(url, L"::/");
> +
> +    if (index)
> +        ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */
> +
> +    SysFreeString(url);
> +}
> +
> +static HRESULT WINAPI WebBrowserEvents2_QueryInterface(IDispatch *iface, REFIID riid, void **v)
> +{
> +    *v = NULL;
> +
> +    if(IsEqualGUID(&DIID_DWebBrowserEvents2, riid) || IsEqualGUID(&IID_IDispatch, riid)) {
> +        *v = iface;
> +        return S_OK;
> +    }
> Normally this should AddRef().

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109136

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +    static DWORD dw = 100;
> +
> +    hres = IWebBrowser2_QueryInterface(info->web_browser->web_browser, &IID_IConnectionPointContainer, (void **)&container);
> +    if (FAILED(hres))
> +        return;
> +
> +    hres = IConnectionPointContainer_FindConnectionPoint(container, &DIID_DWebBrowserEvents2, &point);
> +    IConnectionPointContainer_Release(container);
> +    if (FAILED(hres))
> +        return;
> +
> +    if (init)
> +    {
> +        info->navigate_sink = malloc(sizeof(*(info->navigate_sink)));
> +        info->navigate_sink->iface.lpVtbl = &WebBrowserEvents2Vtbl;
> +        info->navigate_sink->lpVtbl = &WebBrowserEvents2Vtbl;
> Why do you need two fields here?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109140

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +    Dispatch_Release,
> +    NULL,
> +    NULL,
> +    NULL,
> +    WebBrowserEvents2_Invoke
> +};
>
> -        if (index)
> -            ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */
> +static void hook_nav_complete(HHInfo *info, BOOL init)
> +{
> +    IConnectionPointContainer *container;
> +    IConnectionPoint *point;
> +    HRESULT hres;
> +
> +    static DWORD dw = 100;
> Why 100 and static? Usually this works a key to following Unadvise(), initializing it to something is strange.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109141

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> }
>
> -    if (lstrcmpiW(buf, url) > 0)
> +    index = wcsstr(url, L"::/");
> +
> +    if (index)
> +        ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */
> +
> +    SysFreeString(url);
> +}
> +
> +static HRESULT WINAPI WebBrowserEvents2_QueryInterface(IDispatch *iface, REFIID riid, void **v)
> +{
> +    *v = NULL;
> +
> +    if(IsEqualGUID(&DIID_DWebBrowserEvents2, riid) || IsEqualGUID(&IID_IDispatch, riid)) {

Is DIID_* one necessary here? I think normally it's IUnknown and IDispatch.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109142

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
> +
> +static HRESULT WINAPI WebBrowserEvents2_Invoke(IDispatch *iface, DISPID dispIdMember, REFIID riid,
> +        LCID lcid, WORD wFlags, DISPPARAMS *pDispParams, VARIANT *pVarResult,
> +        EXCEPINFO *pExcepInfo, UINT *puArgErr)
> +{
> +    if (dispIdMember == DISPID_NAVIGATECOMPLETE2)
>      {
> -        const WCHAR *index;
> +        WebBrowserEvents2Impl *impl = (WebBrowserEvents2Impl *)iface;
> +        DoSyncContent((HHInfo *)impl->info);
> +    }
> +    return S_OK;
> +}
>
> -        index = wcsstr(url, L"::/");
> +static IDispatchVtbl WebBrowserEvents2Vtbl =
> static const.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109143

[Jacob Czekalla (＠JacobCzekalla)]

On Tue Jul  8 16:51:48 2025 +0000, Nikolay Sivov wrote:
> Why do you need two fields here?
> I did it for IDispatch_Release and IDispatch_AddRef. I realize I should be passing them my iface instead so I do not need the two fields. I will remove info->navigate_sink->lpVtbl.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109153

[Jacob Czekalla (＠JacobCzekalla)]

On Tue Jul  8 16:57:27 2025 +0000, Nikolay Sivov wrote:
> Why 100 and static? Usually this works a key to following Unadvise(),
> initializing it to something is strange.
> That is how it was done in test_ConnectionPoint in ieframe/tests/webbrowser.c so I thought it should have it. It is probably better to store it in my WebBrowserEvents2Impl struct for when I release it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109155