UI: Updated Windows File Registration #107013
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#107013
Loading…
Reference in New Issue
No description provided.
Delete Branch "Harley/blender:RegisterNew"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Windows file associations using ProgID, needed because of the launcher.
This fixes "pin to taskbar" and Recent Documents lists, allow per-
version jump lists and an "Open with" list with multiple versions.
This is mostly code by @LazyDodo from here: https://archive.blender.org/developer/D13587
When we introduced the launcher it created a few problems:
Additionally with modern blender versions being able to install side-by-side, it's hard to even guess what specific blender version is currently configured to launch blender. This PR changes the registration in the following ways:
This also adds an "unregister" option to help cleanup unwanted cruft and allow more control when dealing with multiple blender versions. It also allows these processes to happen for the current user or for all users. The latter will very likely trigger a User Access Control prompt.
This PR alters and adds some command-line arguments:
-r
is altered so that it registers only as current user.-ra
registers for all users, needed for some network installations-u
unregisters as current user-ua
unregisters for all users, needed for some network installationsWe would need to alter the installer so that it calls "-ra" when installing and "-ua" upon uninstallation.
60911fd0d1
to2b09300fb9
2b09300fb9
todcd0d4e55a
dcd0d4e55a
to0ebb8ec902
WIP: Updated Windows File Registrationto UI: Updated Windows File RegistrationUI: Updated Windows File Registrationto WIP: UI: Updated Windows File RegistrationTo do this right it looks like more options need to be added.
We'd need a user-facing checkmark for "All Users", disabled by default. The operators would probably have to ShellExecuteEx ourselves with "runas" (if all users) and WaitForSingleObject in order to report success or failure. The installer would have to use "-ra" and then we can have the uninstall use "-ua".
That was a much longer conversation in person, nice summary!
f63a9486d4
to5efb87545e
@LazyDodo - How's this look?
It does exactly what you think, initiating UAC for All Users, reports success and failure properly, etc.
That doesn't look like the greatest UI? Then again, I'm not on the UI team, so take any criticism here with a grain of salt, and don't by shy telling me to mind my own business.
how about .... (pardon my ms-paint)
No, will play around a bit. The spoiler seems to be the checkbox. When the buttons are on the same row the checkmark looks to be applying to just one of them. It does a bit with mine too of course.
Will keep playing.
But have to say having it do the UAC thing feels pretty awesome.
WIP: UI: Updated Windows File Registrationto UI: Updated Windows File Registrationb78291f2f7
to9ee4b64ee9
mostly small nitpickings, lgtm for landing in 4.0 without another round of review from my end.
@ -1195,0 +1210,4 @@
if (!SUCCEEDED(hr))
return;
/* Find the current executable, and see if it's blender.exe if not bail out. */
Nitpicking, especially since i'm pretty sure this is my code, if we move the check for
blender.exe
beforeSHGetPropertyStoreForWindow
we can save our selves aSHGetPropertyStoreForWindow/Release
call when things "go bad"@ -1195,0 +1218,4 @@
return;
}
/* Set the launcher as the shell command so the console window will not flash.
This can move inside the
if (SUCCEEDED(hr)) {
branch no need to execute it if we're not gonna use it.@ -52,3 +51,2 @@
}
if (!background) {
MessageBox(0, "Could not register file extension.", "Blender error", MB_OK | MB_ICONERROR);
printf("%s\n", message);
would
fprintf(stderr,
be nicer here?@ -62,3 +74,2 @@
HKEY root = 0;
BOOL usr_mode = false;
DWORD dwd = 0;
HKEY hkey = 0;
hkey_progid
Unregistering seems pretty obscure, I can't think of a situation where I would do that from within Blender in practice. But I guess why not.
@ -248,6 +253,8 @@ GHOST_TTrackpadInfo GHOST_WindowWin32::getTrackpadInfo()
GHOST_WindowWin32::~GHOST_WindowWin32()
{
unregisterWindowAppUserModelProperties();
Put inside
if (m_hWnd) {
.@ -1195,0 +1203,4 @@
void GHOST_WindowWin32::registerWindowAppUserModelProperties()
{
IPropertyStore *pstore;
char BlPath[MAX_PATH];
Use snake_case for variables.
@ -1195,0 +1207,4 @@
wchar_t shell_command[MAX_PATH];
HRESULT hr = SHGetPropertyStoreForWindow(m_hWnd, IID_PPV_ARGS(&pstore));
if (!SUCCEEDED(hr))
Always use {}.
@ -643,3 +641,1 @@
split.alignment = 'RIGHT'
split.label(text="")
split.operator("preferences.associate_blend", text="Make Default")
layout.label(text="Open blend files with this installation")
installation -> Blender version
@ -137,0 +140,4 @@
char BlPath[MAX_PATH];
char InstallDir[FILE_MAXDIR];
char SysDir[FILE_MAXDIR];
const char* ThumbHandlerDLL;
Code style is also off here, no snake_case, wrong location for
*
.@ -235,6 +342,41 @@ bool BLI_windows_external_operation_execute(const char *filepath, const char *op
return ShellExecuteExW(&shellinfo);
}
bool BLI_windows_execute_self(const char* parameters, bool wait, bool elevated, bool silent)
const bool
for arguments.@ -281,0 +300,4 @@
if (all_users && BLI_windows_execute_self("-ua", true, true, true)) {
BKE_report(op->reports, RPT_INFO, "File association unregistered");
WM_cursor_wait(false);
MessageBox(
I don't think these operations need a messagebox. Maybe on failure, but not on success.
@ -5819,0 +5821,4 @@
prop = RNA_def_property(srna, "register_all_users", PROP_BOOLEAN, PROP_NONE);
RNA_def_property_boolean_sdna(prop, NULL, "uiflag", USER_REGISTER_ALL_USERS);
RNA_def_property_ui_text(prop, "Register for All Users", "Registration/Unregistration for All Users requires elevated privileges");
The name or description does not indicate what is being registered.
The description should be a proper sentence:
@ -631,1 +630,4 @@
BLI_args_print_arg_doc(ba, "-r");
BLI_args_print_arg_doc(ba, "-ra");
BLI_args_print_arg_doc(ba, "-u");
BLI_args_print_arg_doc(ba, "-ua");
Use longer, less obscure argument names, and only keep
-r
as an alternative abbreviation. I don't think we should take a single letteru
for a relatively unimportant feature.Used to be obscure, but not anymore, with blender 4.0, 4.1, 4.2 now installing and registering side by side, you'd want a way to get rid of the 4.0 registration if you decide to remove it from your system. Sure the MSI installer will take care of that, but it only covers part of our user base and the remaining part of the user base may not be tech savvy enough to navigate the CLI to do it.
Actually, this patch doesn't seem to make any changes to the installer as far as I can tell. Should it now do anything to support the unregister and app ID with the version in it?
Also maybe the MSIX for the Windows store needs some update?
that part of the installer lives in SVN (
win64_vc15\package\installer_wix\WIX.template
) so review of that bit will be a bit challenging. Probably easier to track updating all installers and stores for this in a separate ticket? since this has to land first before we can do any of the other work.@blender-bot build
@blender-bot build
Looks good to me now by itself, assuming MSI and MSIX changes will come after this.
I don't know the exact process of creating an MSIX package for local testing, but can help figure it out if needed.
Following is our current proposed WIX.template with changes to call with " --register-allusers" on installation, and " --unregister-allusers" during uninstall. As mentioned this would be done outside this PR.
I have to admit, i wasn't involved/consulted on the windows store, I'm not even sure if the secret sauce for that one is publicly available. There's some resource files in git over at
release\windows\msix\
which i'm not entirely sure how to use.Microsoft Store installations are more problematic than I realized. These are so sandboxed that these changes to the registry can't work. Registering file associations this way or from the command-line just silently fail because the OS pretends success.
Therefore this PR now adds a function to detect Store installations so that the buttons can disable with nice tooltip explanations:
I think it is better to show the items disabled rather than hiding them as this seems less likely to result in bug reports.
The detection of Store installations is only doing so by examining the executable path for "\WindowsApps" which seems weak but I haven't (yet) found anything better.
It seems file type association is already in
release/windows/msix/AppxManifest.xml.template
. Is that not working?If that works, I'd still suggest to hide this panel entirely, or at least replace the buttons by some text explaining things.
That should work for associating .blend files with the executable, and we have had no reports of it not working. But doing it that way cannot install the thumbnailer and it looks like there is no way of doing so while the program runs in this sandboxed environment. We have had at least one report of that, #107921.
Hiding the panel entirely was Ray's first thought too, and I don't mind doing that. My idea above with the disabled buttons and the explanatory disabled hints was this explains the situation better so we wouldn't get complaints of "Youtube says these buttons should be here". But it isn't a very good explanation that can only be seen by hovering so I'm probably wrong on that.
Hiding the panel wouldn't have stopped #107921 so probably "replace the buttons by some text explaining things" might work.
My gut instincts are often wrong, i'm with @Harley on this now, explaining why it's disabled will generate fewer support tickets than flat out hiding it.
Thumbnails should be possible, it seems Krita does it?
https://invent.kde.org/graphics/krita/-/blob/master/packaging/windows/msix/manifest.xml.in
https://learn.microsoft.com/en-us/windows/apps/desktop/modernize/desktop-to-uwp-extensions#show-file-contents-in-a-thumbnail-image-within-file-explorer
I'm fine with some text until that is working, but maybe it's faster to make the thumbnail work.
Those links do imply that it might be possible for the store installer to do both the registration of blend files and install the thumbnailer, because it runs with elevated privileges. But it would still not be possible to do or change these things within Blender since it is sandboxed while running.
So the above links might solve #107921 but we are still stuck with having to hide or explain these buttons to the users who have installed via the MS store. And I guess if there is chance we want to detect this from python I should add... a readonly dynamic property? bpy.context.preferences.system.is_msstore_installation or similar.
Now also showing different text - "Not applicable to MS Store installation"- when in this state:
Can this be just text, no grayed out buttons? Something like:
That works quite well.
Note that I might skive off early today. Its a national holiday and wife is home.
Just some nitpicks, for which fixes don't need further review from me.
@ -665,6 +668,22 @@ static void rna_UserDef_viewport_lights_update(Main *bmain, Scene *scene, Pointe
rna_userdef_update(bmain, scene, ptr);
}
static int rna_register_all_users_editable(struct PointerRNA *UNUSED(ptr), const char **r_info)
This is unnecessary now, and settings that do nothing should generally still be editable.
@ -668,0 +681,4 @@
static bool rna_userdef_is_msstore_installation_get(PointerRNA *UNUSED(ptr))
{
return BLI_windows_is_store_install();
Needs
# ifdef WIN32
@ -5819,0 +5848,4 @@
"is_msstore_installation",
false,
"Is MS Store Installation",
"Whether this blender installation is a sandboxed Microsoft Store version.");
I'd avoid the abbrevation and instead use
is_microsoft_store_install
and"Microsoft Store Install"
2fb00457e1
to7de823924c
@blender-bot build
@blender-bot build windows