more fixes around DialogEx focus handling and default pushbuttons

I found some more areas inside existing walk code where it is trying
to re-implement various dialog manager features that we want to
avoid when using DialogEx:

* Form's WM_ACTIVATE handler tries to save and restore the currently
  focused widget. This competes with the dialog manager, so I added
  an override in DialogEx to only track the active form.
* I made WM_NEXTDLGCTL use SendMessage whenever possible by flagging
  when we're handling focus messages and only switching to PostMessage
  when we risk re-entering. We set that flag in WM_SETFOCUS and
  WM_KILLFOCUS handlers.
* I prevent layout from forcibly focusing widgets when the enclosing
  form is a DialogEx; we want that dealt with elsewhere.
* I created overrides for setCtrlID and clearCtrlID in PushButton that
  also set/clear BS_DEFPUSHBUTTON and re-focus the default pushbutton.
  This is necessary because setting focus to the default pushbutton
  typically happens when a dialog template is processed at creation
  time; of course, we're attaching widgets long after that has occurred.
  We also want to clear BS_DEFPUSHBUTTON once detaching widgets from
  the form, otherwise invalid state remains cached internally to Win32.

Updates tailscale/corp#23789

Signed-off-by: Aaron Klotz <[email protected]>

Author: dblohm7

Diff for: dialogex.go

@@ -209,11 +209,19 @@ func (dlg *DialogEx) dlgProc(hdlg win.HWND, msg uint32, wParam, lParam uintptr)
 	case win.WM_NOTIFY:
 		dlg.FormBase.WndProc(hdlg, msg, wParam, lParam)
 		return true
-	case win.WM_SETTINGCHANGE, win.WM_THEMECHANGED, win.WM_SETTEXT, win.WM_WINDOWPOSCHANGED, win.WM_SYSCOLORCHANGE, win.WM_DPICHANGED, win.WM_GETMINMAXINFO:
-		dlg.FormBase.WndProc(hdlg, msg, wParam, lParam)
-		// Once we handle these messages, we return false so that DefDlgProc also receives them.
+	case win.WM_ACTIVATE:
+		// FormBase's WM_ACTIVATE handler does some goofy stuff with focus that we
+		// don't want, so we provide our own implementation.
+		switch win.LOWORD(uint32(wParam)) {
+		case win.WA_ACTIVE, win.WA_CLICKACTIVE:
+			activeForm = dlg.AsFormBase()
+		case win.WA_INACTIVE:
+			activeForm = nil
+		}
 		return false
 	default:
+		// Once we handle these messages, we return false so that DefDlgProc also receives them.
+		dlg.FormBase.WndProc(hdlg, msg, wParam, lParam)
 		return false
 	}
 }
@@ -251,13 +259,13 @@ func (dlg *DialogEx) Cancel() {
 // SetFocusNext moves keyboard focus to the next Widget in the dialog's tab
 // sequence.
 func (dlg *DialogEx) SetFocusNext() {
-	win.PostMessage(dlg.hWnd, win.WM_NEXTDLGCTL, 0, 0)
+	dlg.nextDlgCtl(0, 0)
 }
 
 // SetFocusPrev moves keyboard focus to the previous Widget in the dialog's tab
 // sequence.
 func (dlg *DialogEx) SetFocusPrev() {
-	win.PostMessage(dlg.hWnd, win.WM_NEXTDLGCTL, 1, 0)
+	dlg.nextDlgCtl(1, 0)
 }
 
 // SetFocusToWindow sets keyboard focus within dlg to the Window specified by w.
@@ -273,7 +281,7 @@ func (dlg *DialogEx) SetFocusToWindow(w Window) {
 		return
 	}
 
-	win.PostMessage(dlg.hWnd, win.WM_NEXTDLGCTL, uintptr(hwnd), 1)
+	dlg.nextDlgCtl(uintptr(hwnd), 1)
 }
 
 // EnterMode satisfies Modal.
@@ -422,3 +430,21 @@ func (dlg *DialogEx) routeWM_COMMAND(wParam, lParam uintptr) (result bool) {
 	}
 	return false
 }
+
+// ResetDefaultFocus makes dlg re-determine its default pushbutton (if any),
+// and resets keyboard focus to dlg's default Widget.
+func (dlg *DialogEx) ResetDefaultFocus() {
+	// Use the HWND variant, but with a null HWND.
+	dlg.nextDlgCtl(0, 1)
+}
+
+func (dlg *DialogEx) nextDlgCtl(wParam, lParam uintptr) {
+	// Windows's keyboard focus-setting is not re-entrant. If we're already in
+	// the midst of processing a focus change, we need to post this message
+	// instead of sending it.
+	if dlg.handlingFocusChange {
+		win.PostMessage(dlg.hWnd, win.WM_NEXTDLGCTL, wParam, lParam)
+	} else {
+		win.SendMessage(dlg.hWnd, win.WM_NEXTDLGCTL, wParam, lParam)
+	}
+}

Diff for: layout.go

@@ -339,7 +339,10 @@ func applyLayoutResults(results []LayoutResult, stopwatch *stopwatch) error {
 							activeForm, _ := windowFromHandle(hwndForm).(Form)
 
 							if hwndFocused == 0 || form.Handle() == hwndFocused || activeForm != window.Form() {
-								form.AsFormBase().clientComposite.focusFirstCandidateDescendant()
+								// Only set explicit focus if we're not a DialogEx.
+								if _, isDialogEx := form.(DialogExResolver); !isDialogEx {
+									form.AsFormBase().clientComposite.focusFirstCandidateDescendant()
+								}
 							}
 						}()
 					}

Diff for: pushbutton.go

@@ -17,6 +17,7 @@ type PushButton struct {
 	Button
 	contentMargins win.MARGINS
 	layoutFlags    LayoutFlags
+	wantDefault    bool
 }
 
 // NewPushButton creates a new PushButton as a child of parent with its
@@ -30,7 +31,7 @@ func NewPushButton(parent Container) (*PushButton, error) {
 type PushButtonOptions struct {
 	LayoutFlags  LayoutFlags // LayoutFlags to be used by the PushButton.
 	PredefinedID int         // When non-zero, must be one of the predefined control IDs <= [win.IDCONTINUE].
-	Default      bool        // When true, the PushButton will be initially created as a default PushButton.
+	Default      bool        // When true, the PushButton will set itself as the default PushButton for the Form it resides in.
 }
 
 // NewPushButtonWithOptions creates a new PushButton as a child of parent
@@ -42,20 +43,14 @@ func NewPushButtonWithOptions(parent Container, opts PushButtonOptions) (*PushBu
 
 	pb := &PushButton{
 		layoutFlags: opts.LayoutFlags,
-	}
-
-	style := uint32(win.WS_TABSTOP | win.WS_VISIBLE)
-	if opts.Default {
-		style |= win.BS_DEFPUSHBUTTON
-	} else {
-		style |= win.BS_PUSHBUTTON
+		wantDefault: opts.Default,
 	}
 
 	if err := InitWidget(
 		pb,
 		parent,
 		"BUTTON",
-		style,
+		win.WS_TABSTOP|win.WS_VISIBLE,
 		0); err != nil {
 		return nil, err
 	}
@@ -86,6 +81,65 @@ func (pb *PushButton) SetImageAboveText(value bool) error {
 	return pb.SetImage(pb.image)
 }
 
+func (pb *PushButton) isDefault() bool {
+	return pb.hasStyleBits(win.BS_DEFPUSHBUTTON)
+}
+
+func (pb *PushButton) setCtrlID(ids ctrlIDAllocator) {
+	var id uint16
+	if !pb.usesPredefinedID {
+		id = ids.allocCtrlID()
+		pb.setCtrlIDInternal(id)
+	}
+
+	if pb.wantDefault {
+		if dlgExResolver, ok := pb.ancestor().(DialogExResolver); ok {
+			if pb.usesPredefinedID {
+				// We need to know the existing predefined ID.
+				id = pb.getCtrlID()
+			}
+
+			// Ensure BS_DEFPUSHBUTTON is set.
+			pb.setAndClearStyleBits(win.BS_DEFPUSHBUTTON, win.BS_PUSHBUTTON)
+
+			dlgEx := dlgExResolver.AsDialogEx()
+			// IDs are being assigned by FormBase code after the DialogEx has already
+			// been created, so we need to inform the dialog that our control ID
+			// represents the default button.
+			win.SendMessage(dlgEx.hWnd, win.DM_SETDEFID, uintptr(id), 0)
+			dlgEx.SetFocusToWindow(pb)
+		}
+	}
+}
+
+func (pb *PushButton) clearCtrlID(ids ctrlIDAllocator) {
+	var id uint16
+	if !pb.usesPredefinedID {
+		id = pb.setCtrlIDInternal(0)
+		ids.freeCtrlID(id)
+	}
+
+	if pb.wantDefault {
+		if dlgExResolver, ok := pb.ancestor().(DialogExResolver); ok {
+			if pb.usesPredefinedID {
+				// We need to know the existing predefined ID.
+				id = pb.getCtrlID()
+			}
+
+			// Ensure BS_DEFPUSHBUTTON is cleared.
+			pb.setAndClearStyleBits(win.BS_PUSHBUTTON, win.BS_DEFPUSHBUTTON)
+
+			dlgEx := dlgExResolver.AsDialogEx()
+			// See whether the dialog's current default ID is ours...
+			result := uint32(win.SendMessage(dlgEx.hWnd, win.DM_GETDEFID, 0, 0))
+			if win.HIWORD(result)&win.DC_HASDEFID != 0 && win.LOWORD(result) == id {
+				// ...and if so, clear it.
+				win.SendMessage(dlgEx.hWnd, win.DM_SETDEFID, uintptr(0), 0)
+			}
+		}
+	}
+}
+
 func (pb *PushButton) ensureProperDialogDefaultButton(hwndFocus win.HWND) {
 	widget := windowFromHandle(hwndFocus)
 	if widget == nil {

Diff for: window.go

@@ -455,6 +455,7 @@ type WindowBase struct {
 	calcTextSizeInfo2TextSize   map[calcTextSizeInfo]Size // in native pixels
 	suspended                   bool
 	enabled                     bool
+	handlingFocusChange         bool
 	acc                         *Accessibility
 	themes                      map[string]*Theme
 	menuSharedMetricsInitialDPI *menuSharedMetrics
@@ -2550,6 +2551,11 @@ func (wb *WindowBase) WndProc(hwnd win.HWND, msg uint32, wParam, lParam uintptr)
 		wb.publishMouseWheelEvent(&wb.mouseWheelPublisher, wParam, lParam)
 
 	case win.WM_SETFOCUS, win.WM_KILLFOCUS:
+		wb.handlingFocusChange = true
+		defer func() {
+			wb.handlingFocusChange = false
+		}()
+
 		switch wnd := wb.window.(type) {
 		// case *splitterHandle:
 		// nop