[PATCH v2 0/1] MR5012: msi: Disable Browse Button when feature has no associated directory

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56289

--
  v2: msi: Disable Browse Button when feature has no associated directory
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012

[PATCH v2 1/1] msi: Disable Browse Button when feature has no associated directory

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56289
---
 dlls/msi/dialog.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/dlls/msi/dialog.c b/dlls/msi/dialog.c
index ba8ca526266..7defadebde8 100644
--- a/dlls/msi/dialog.c
+++ b/dlls/msi/dialog.c
@@ -644,6 +644,11 @@ static void dialog_handle_event( msi_dialog *dialog, const WCHAR *control, const
         SetWindowTextW( ctrl->hwnd, path );
         free( path );
     }
+    else if ( !wcscmp ( attribute, L"SelectionBrowseButton" ) )
+    {
+        const WCHAR *directory = MSI_RecordGetString( rec, 1 );
+        EnableWindow(ctrl->hwnd, directory != NULL);
+    }
     else
     {
         FIXME("Attribute %s not being set\n", debugstr_w(attribute));
@@ -1007,11 +1012,32 @@ end:
     return hBitmap;
 }
 
+static UINT dialog_find_browse_event( MSIRECORD *rec, void *param )
+{
+    msi_dialog *dialog = param;
+    LPCWSTR condition, event;
+    UINT r;
+
+    condition = MSI_RecordGetString( rec, 5 );
+    r = MSI_EvaluateConditionW( dialog->package, condition );
+    if (r == MSICONDITION_TRUE || r == MSICONDITION_NONE)
+    {
+        event = MSI_RecordGetString( rec, 3 );
+        if (!wcscmp(event, L"SelectionBrowse"))
+        {
+            return ERROR_SUCCESS;
+        }
+    }
+    return ERROR_NOT_FOUND;
+}
+
 static UINT dialog_button_control( msi_dialog *dialog, MSIRECORD *rec )
 {
     struct control *control;
     UINT attributes, style, cx = 0, cy = 0, flags = 0;
     WCHAR *name = NULL;
+    MSIQUERY *view;
+    UINT r;
 
     TRACE("%p %p\n", dialog, rec);
 
@@ -1056,6 +1082,19 @@ static UINT dialog_button_control( msi_dialog *dialog, MSIRECORD *rec )
         else ERR("Failed to load bitmap %s\n", debugstr_w(name));
     }
 
+    r = MSI_OpenQuery( dialog->package->db, &view,
+                         L"SELECT * FROM `ControlEvent` WHERE `Dialog_` = '%s' AND `Control_` = '%s' ORDER BY `Ordering`",
+                         dialog->name, control->name );
+    if (r != ERROR_SUCCESS)
+        return r;
+
+    r = MSI_IterateRecords( view, 0, dialog_find_browse_event, dialog );
+    if (r == ERROR_SUCCESS)
+    {
+        event_subscribe(dialog, L"SelectionPath", control->name, L"SelectionBrowseButton");
+    }
+    msiobj_release( &view->hdr );
+
     free( name );
     return ERROR_SUCCESS;
 }
-- 
GitLab

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

[Fabian Maurer (＠DarkShadow44)]

On Mon Feb  5 12:24:35 2024 +0000, Hans Leidekker wrote:
> It looks like you're working around the real bug. We shouldn't have to
> dig into the database at this point.
> FWIF, there is only a `ControlCondition` that can hide the Browse button, so I don't think the installer sets the button to disabled manually.

I pushed an update, is this better?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_60034

[Hans Leidekker (＠hans)]

On Mon Feb  5 12:24:35 2024 +0000, Fabian Maurer wrote:
> FWIF, there is only a `ControlCondition` that can hide the Browse
> button, so I don't think the installer sets the button to disabled manually.
> I pushed an update, is this better?
> It's still too specific. The functions you are modifying (dialog_handle_event / dialog_button_control) are generic, they shouldn't have any special handling for browse buttons.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_60035

[Fabian Maurer (＠DarkShadow44)]

On Mon Feb  5 12:41:21 2024 +0000, Hans Leidekker wrote:
> It's still too specific. The functions you are modifying
> (dialog_handle_event / dialog_button_control) are generic, they
> shouldn't have any special handling for browse buttons.
> How else should I handle this? I tried editing the msi with "InstEd", and as soon as the button-event is no longer "SelectionBrowse", the button is no longer disabled when switching features. Otherwise it seems to be just a normal button.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_60076

[Hans Leidekker (＠hans)]

On Mon Feb  5 20:55:03 2024 +0000, Fabian Maurer wrote:
> How else should I handle this? I tried editing the msi with "InstEd",
> and as soon as the button-event is no longer "SelectionBrowse", the
> button is no longer disabled when switching features. Otherwise it seems
> to be just a normal button.
> It seems like you found a good lead. The handler for SelectionBrowse is event_spawn_dialog(), so perhaps you can follow that path and see which part causes the button to be enabled?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_60083

[Fabian Maurer (＠DarkShadow44)]

On Mon Feb  5 21:48:02 2024 +0000, Hans Leidekker wrote:
> It seems like you found a good lead. The handler for SelectionBrowse is
> event_spawn_dialog(), so perhaps you can follow that path and see which
> part causes the button to be enabled?
> Sorry, I have no idea what you mean. When the button is pressed, it's already too late to disable it. And it's enabled because we never disable it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_60085

[Fabian Maurer (＠DarkShadow44)]

On Mon Feb  5 21:55:43 2024 +0000, Fabian Maurer wrote:
> Sorry, I have no idea what you mean. When the button is pressed, it's
> already too late to disable it. And it's enabled because we never
> disable it.
> How should I continue here? I don't see a different way to do this

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_77837

[Fabian Maurer (＠DarkShadow44)]

Can someone tell me what I should do here?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5012#note_81119