[PATCH v2 0/1] MR8526: hhctrl.ocx: Selection of treeview items in the content tab should reflect web browser page.
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. -- v2: hhctrl.ocx: Selection of treeview items in the content tab should reflect web browser page. https://gitlab.winehq.org/wine/wine/-/merge_requests/8526
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 | 167 ++++++++++++++++++++++++++++++++++++--- dlls/hhctrl.ocx/hhctrl.h | 14 ++++ 2 files changed, 170 insertions(+), 11 deletions(-) diff --git a/dlls/hhctrl.ocx/help.c b/dlls/hhctrl.ocx/help.c index 413659c7097..bfcf80b4f1a 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,126 @@ 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(&IID_IDispatch, riid)) + { + *v = iface; + IDispatch_AddRef(iface); + return S_OK; + } + + return E_NOINTERFACE; +} + +static inline WebBrowserEvents2Impl *impl_from_IDispatch(IDispatch *iface) +{ + return (WebBrowserEvents2Impl *)iface; +} + +static ULONG WINAPI WebBrowserEvents2_AddRef(IDispatch *iface) +{ + WebBrowserEvents2Impl *impl = impl_from_IDispatch(iface); + return InterlockedIncrement(&impl->ref); +} + +static ULONG WINAPI WebBrowserEvents2_Release(IDispatch *iface) +{ + WebBrowserEvents2Impl *impl = impl_from_IDispatch(iface); + ULONG ref = InterlockedDecrement(&impl->ref); + + if (!ref) + free(impl); + return ref; +} + +static HRESULT WINAPI WebBrowserEvents2_GetTypeInfoCount(IDispatch *iface, UINT *pctinfo) +{ + return E_NOTIMPL; +} + +static HRESULT WINAPI WebBrowserEvents2_GetTypeInfo(IDispatch *iface, UINT iTInfo, LCID lcid, + ITypeInfo **ppTInfo) +{ + return E_NOTIMPL; +} + +static HRESULT WINAPI WebBrowserEvents2_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *rgszNames, + UINT cNames, LCID lcid, DISPID *rgDispId) +{ + return E_NOTIMPL; +} + +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; +} + +static const IDispatchVtbl WebBrowserEvents2Vtbl = +{ + WebBrowserEvents2_QueryInterface, + WebBrowserEvents2_AddRef, + WebBrowserEvents2_Release, + WebBrowserEvents2_GetTypeInfoCount, + WebBrowserEvents2_GetTypeInfo, + WebBrowserEvents2_GetIDsOfNames, + WebBrowserEvents2_Invoke +}; + +static void hook_nav_complete(HHInfo *info, BOOL init) +{ + IConnectionPointContainer *container; + IConnectionPoint *point; + HRESULT hres; - index = wcsstr(url, L"::/"); + hres = IWebBrowser2_QueryInterface(info->web_browser->web_browser, &IID_IConnectionPointContainer, (void **)&container); + if (FAILED(hres)) + return; - if (index) - ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */ + 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->info = (struct HHInfo *)info; + info->navigate_sink->ref = 1; + IConnectionPoint_Advise(point, (IUnknown *)info->navigate_sink, &info->navigate_sink->cookie); + } + else + { + IConnectionPoint_Unadvise(point, info->navigate_sink->cookie); + IDispatch_Release(&info->navigate_sink->iface); } - SysFreeString(url); + IConnectionPoint_Release(point); } /* Size Bar */ @@ -577,9 +689,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 +735,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 +1022,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 +1300,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 +1929,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..eeafaebb3a8 100644 --- a/dlls/hhctrl.ocx/hhctrl.h +++ b/dlls/hhctrl.ocx/hhctrl.h @@ -181,6 +181,18 @@ typedef struct { HWND hwndWindow; } WebBrowserContainer; +struct HHInfo; + +typedef struct WebBrowserEvents2Impl +{ + IDispatch iface; + IDispatchVtbl *lpVtbl; + + struct HHInfo *info; + LONG ref; + DWORD cookie; +} WebBrowserEvents2Impl; + typedef struct { WebBrowserContainer *web_browser; @@ -203,6 +215,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) commented about dlls/hhctrl.ocx/help.c:
- if (index) - ActivateContentTopic(info->tabs[TAB_CONTENTS].hwnd, index + 3, info->content); /* skip over ::/ */ + 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->info = (struct HHInfo *)info; + info->navigate_sink->ref = 1; + IConnectionPoint_Advise(point, (IUnknown *)info->navigate_sink, &info->navigate_sink->cookie); + }
This creates problems with lifetime management. Referencing window data in it, which is not refcounted, is not safe usually. I think webbrowser container is a better place for it. When it's time to detach the container, you would Unadvise and Release, this will cleanup properly I think. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109301
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(&IID_IDispatch, riid))
This should include IID_IUnknown too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109304
Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
citer = (ContentItem *) user_data; name = citer->name; local = citer->local; + if (is_current_page(info, local)) + return 0; +
Is this related to handling navigation event? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109302
Nikolay Sivov (@nsivov) commented about dlls/hhctrl.ocx/help.c:
+ *v = NULL; + + if(IsEqualGUID(&IID_IDispatch, riid)) + { + *v = iface; + IDispatch_AddRef(iface); + return S_OK; + } + + return E_NOINTERFACE; +} + +static inline WebBrowserEvents2Impl *impl_from_IDispatch(IDispatch *iface) +{ + return (WebBrowserEvents2Impl *)iface; +} Please check how it's done in other places.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109303
On Wed Jul 9 10:53:32 2025 +0000, Nikolay Sivov wrote:
Is this related to handling navigation event? NavigateComplete2 -> DoSync -> ActivateContentTopic -> TVM_SELECTITEM -> OnTopicChange -> NavigateToChm -> NavigateComplete2
This check prevents NavigateComplete2 from getting called again due to itself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109317
On Wed Jul 9 12:57:45 2025 +0000, Jacob Czekalla wrote:
NavigateComplete2 -> DoSync -> ActivateContentTopic -> TVM_SELECTITEM -> OnTopicChange -> NavigateToChm -> NavigateComplete2 This check prevents NavigateComplete2 from getting called again due to itself. You don't get notification if item hasn't changed in TVM_SELECTITEM.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109328
On Wed Jul 9 14:23:04 2025 +0000, Nikolay Sivov wrote:
You don't get notification if item hasn't changed in TVM_SELECTITEM. The first time DoSync is called it updates the treeview selection, however I don't want it to navigate to the page because it is already navigated to and It is just syncing the treeview selection so it matches the loaded page. Another call NavigateToChm would be unnecessary.
The logic for navigation history is also getting in the way. It will add another reference to itself to the history. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8526#note_109331
participants (3)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Nikolay Sivov (＠nsivov)