Repository navigation
Hide native Windows 11 Start button when using a replacement - #2547
YellowNest wants to merge 34 commits into
Conversation
|
This is super awesome. Glad to see it works nicely. And I guess it should be used for more customizations and improvements related to start button and taskbar itself. Do you plan to work more in this area? I think it would be really appreciated. |
|
I have noticed that enabling custom Start button cause also that TaskView button dissapears. There is just empty place instead of the button. |
|
Another issue is that when I Starting |
|
Thanks for testing this carefully. Both reports were valid and pointed to separate problems in the initial TAP implementation. I pushed the follow-up in The Task View regression came from treating every The restart problem was a separate TAP lifetime issue. Shutdown now restores the XAML properties synchronously, removes the visual-tree callback with The build for the follow-up is here: The two paths that still deserve runtime confirmation are:
And yes, I plan to keep working in this area. Replacing |
Definitely. I'm glad you plan to work on this more. |
|
Thanks, I really appreciate that. I use Open-Shell myself, so contributing here is useful to me too. I care about keeping it working well on current Windows builds, and if I can help take some of the load off, I'm happy to. I also appreciate the trust you're putting in the work. Your reviews and testing are valuable, especially around these Windows 11 edge cases, so I'll keep changes focused and separate like you suggested. I've already started looking at the XAML input side in a separate research branch. The first build is clean, but I'll keep it out of upstream until I've tested the behavior properly. Glad to help. |
I can confirm this works properly now.
I'm now experiencing random Explorer crashes when Exiting Open-Shell and starting it again (using Open-Shell Menu Settings shortcut). This is on 26H2 (Windows Sandbox). I can share the dump eventually. |
|
Thanks for the dump trace. I treated this as a teardown/lifetime problem rather than an accessibility-code failure and reworked the TAP shutdown path. The latest head is The important changes are:
I also checked the current upstream master changes; they don't overlap this code. The full upstream PR build for The critical runtime test now is repeated cycles on 26H2:
If Explorer still crashes, the full dump would be useful. I don't want to paper over this with another timing workaround; the remaining failure, if any, should be traced from the dump. |
Bundle current master with pending PRs Open-Shell#2547, Open-Shell#2548, Open-Shell#2551, Open-Shell#2552, and Open-Shell#2553 for local integration testing. This branch is test packaging only and is not intended for upstream merge.
2fb59e7 to
192b2e3
Compare
|
The crash that I mentioned in #2547 (comment) is actually not related to changes in this PR. I can replicate it with official So far have no clue what could be wrong. |
ge0rdi
left a comment
ge0rdi
left a comment
There was a problem hiding this comment.
I'm sorry. but I have feeling like it gets more complicated with each batch of changes :(
I'd rather keep it simpler (as the idea is rather simple).
I'll try to provide some more comments, just don't have energy for it now.
| ~CWin11StartButtonTap( void ) | ||
| { | ||
| { | ||
| std::unique_lock lock(g_TapMutex); |
There was a problem hiding this comment.
This doesn't feel right. We should rather remove g_Tap reference when we are deactivating TAP (Deactivate method or StopWin11StartButtonTap perhaps).
| std::unordered_map<InstanceHandle, StartElement> m_Elements; | ||
| }; | ||
|
|
||
| static CComPtr<CWin11StartButtonTap> &PublishedTap( void ) |
There was a problem hiding this comment.
There doesn't seem to be any benefit of having such PublishedTap function.
We can just access global tap CComPtr object directly (still the tap lock should be held).
ge0rdi
left a comment
ge0rdi
left a comment
There was a problem hiding this comment.
Few ideas about how to simplify things:
SetSite should allow only one site (that's how UWPSpy and TranslucentTB do it too).
So there will be no need to deal with Deactivate and stuff here.
Dispatch window should be created in TAP object constructor because there 1:1 relation between them.
We may possibly use CWindow class that should hide all the manual handling.
Alternatively, we may consider splitting the class into two - TAP (that will be created by XAML framework and will receive XAML "site") and object that will handle actual XAML tree notifications/changes.
This is how UWPSpy/TranslucentTB are doing it. I think it kind of makes sense to split the responsibilities (though maybe it is not necessary).
When deactivating TAP (this is done only on OpenShell exit) we should:
- stop posting any "apply" messages to dispatch
- drain all "apply" messages in dispatch window
- restore changes we did in XAML tree
- call Unadvise
After that TAP object should be destroyed and we don't need to do anything special (only destroy dispatch window that should be no longer processing anything).
I'd also reconsider the need for m_LifecycleMutex as with the simplified workflow I think it may ne not necessary anymore.
This may even make sense because now the TAP class is called So it would make sense to have generic TAP class, that will then instantiate object that will work with XAML tree. |
|
|
||
| static void CleanStartMenuDLL( void ) | ||
| { | ||
| StopWin11StartButtonMonitor(); |
There was a problem hiding this comment.
I think we may need to call this after UpdateTaskBars(TASKBAR_CLEAR); later in this method.
Because that will call UpdateWin11StartButtonMonitor and that will initialize the TAP thing again (unnecessarily).
I wanted to say |
| HRESULT hr = tap->Deactivate(); | ||
| if (FAILED(hr)) | ||
| LogToFile(STARTUP_LOG, L"Win11StartButtonTap: deactivate failed 0x%08X", hr); | ||
| } |
There was a problem hiding this comment.
If we have tap object, it means there was successful call to InitializeXamlDiagnosticsEx and that means our DLL load count was increased by 1 (it seems that API causes DLL load but it never does unload).
So we can do FreeLibrary(GetThisModule()); here.
It is not nice but with this StartMenuHelper64 will unload on Open-Shell exit. This is basically what UWPSpy does.
I'm still not sure whether it is worth it.
The only benefit would probably be that update to new version won't require Explorer restart (no DLLs locked).
Dunno :(
| return 0; | ||
| } | ||
|
|
||
| static DWORD WINAPI ConnectThread( LPVOID param ) |
There was a problem hiding this comment.
This function can be rearranged a bit and then there will be no need for FinishConnectThread (with rather arbitrary arguments):
static DWORD WINAPI ConnectThread( LPVOID param )
{
HMODULE moduleReference = (HMODULE)param;
HRESULT last = E_FAIL;
HMODULE runtime = LoadLibraryEx(L"Windows.UI.Xaml.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (runtime)
{
InitXamlDiagnosticsEx_t init = (InitXamlDiagnosticsEx_t)GetProcAddress(runtime, "InitializeXamlDiagnosticsEx");
if (init)
{
HMODULE module = GetThisModule();
wchar_t dllPath[MAX_PATH];
if (module && GetModuleFileName(module, dllPath, _countof(dllPath)))
{
static const wchar_t* endpoints[] = { L"VisualDiagConnection1", L"VisualDiagConnection2" };
for (int retry = 0; retry < 8 && g_StartButtonActive; retry++)
{
for (int i = 0; i < _countof(endpoints); i++)
{
last = init(endpoints[i], GetCurrentProcessId(), NULL, dllPath, CLSID_OpenShellStartButtonTap, NULL);
if (SUCCEEDED(last))
{
LogToFile(STARTUP_LOG, L"Win11StartButton: connected using %s", endpoints[i]);
break;
}
}
if (SUCCEEDED(last))
break;
Sleep(500);
}
}
}
FreeLibrary(runtime);
}
if (!SUCCEEDED(last))
{
g_ConnectStarted = false;
LogToFile(STARTUP_LOG, L"Win11StartButton: connection failed 0x%08X", last);
}
FreeLibraryAndExitThread(moduleReference, 0);
return 0;
}
|
In my opinion there shouldn't be references or checks for Windows 11 in variables and filenames as XAML may still be used in the future for a possible Windows 12. I think it is always better to check for the presence of the feature (XAML) rather then the Windows version. |
I agree.
I'm not sure we really support those. I'd say many other things will break there.
That's good idea. |
| if (!module || !GetModuleFileName(module, dllPath, _countof(dllPath))) | ||
| return FinishConnectThread(moduleReference, runtime, false); | ||
|
|
||
| const wchar_t *endpoints[] = { L"VisualDiagConnection1", L"VisualDiagConnection2" }; |
There was a problem hiding this comment.
Other tools (UWPSpy / TranslucentTB) just add numeric suffix based on attempt number.
So maybe we can have just single loop and use VisualDiagConnection%d with retry+1?
This branch has not been deployed
3 participants
Summary
Hide the native Windows 11 Start button visuals while Open-Shell's replacement Start button is enabled.
Windows 11 renders the Start button through XAML, so the older HWND-based hiding logic cannot remove the native glyph. This caused custom Open-Shell buttons to be drawn on top of the Windows Start icon.
The change uses the Windows XAML diagnostics visual tree to:
All taskbarssettingThe replacement button itself is not moved or resized by this code, so Windows remains responsible for centered taskbar layout.
Verification
Tested on Windows 11 25H2 with centered taskbar:
Replace Start buttonrestores the native Windows Start button.GitHub Actions
Buildcompleted successfully for the current heade9039e3:https://github.com/Open-Shell/Open-Shell-Menu/actions/runs/37805091803
The lifecycle implementation avoids loading StartMenuHelper and initializing XAML Diagnostics when the replacement Start button is disabled.
Repeated Explorer exit/restart and complete injected DLL unloading still require runtime verification. A successful build does not establish those behaviors.
Fixes #1631