ncruces / zenity

Zenity dialogs for Golang, Windows, macOS
https://pkg.go.dev/github.com/ncruces/zenity
MIT License
736 stars 37 forks source link

[macOS] Dialogs don't get focus automatically #28

Closed lonnywong closed 2 years ago

lonnywong commented 2 years ago

Test code:

package main

import (
    "fmt"
    "github.com/ncruces/zenity"
)

func main() {
    file, err := zenity.SelectFile(zenity.Title("Test focus on MacOS"))
    if err != nil {
        fmt.Println(err)
    }
    fmt.Println(file)
}

Run in iTerm2, the SelectFile dialog does not get focus unless click the title bar. Similar to https://discussions.apple.com/thread/2548172

Run in native Terminal, the SelectFile dialog get focus with one click. Is it possible to get focus automatically?

ncruces commented 2 years ago

Yes, this is a known issue.

The problem has to do with this code snippet at the top of every script:

var app = Application.currentApplication()
app.includeStandardAdditions = true
app.activate()

The issue is that Application.currentApplication() returns the osascript application, which does not have a UI, and can't be "activated".

The solution might be to tell the currently active application (which might be Terminal or iTerm2) to show the dialog instead.

This is likely what you want but:

A more complicated alternative is to compile a new application on the fly as I do for Progress, but that involves temporary files.

There are no great solutions here.

lonnywong commented 2 years ago

Thanks for your help.

Is there a way to find the current active app ?

I'm developing a command line tools. It should be run in a terminal emulator, e.g. iTerm2, tabby, Terminal, etc.

ncruces commented 2 years ago

If you want to fork this, I think you can replace the above snippet with:

var app = Application(Application('System Events').processes.whose({frontmost: true})[0].unixId())
app.includeStandardAdditions = true

And then run:

go generate ./...
lonnywong commented 2 years ago

If you want to fork this, I think you can replace the above snippet with:

var app = Application(Application('System Events').processes.whose({frontmost: true})[0].unixId())
app.includeStandardAdditions = true

And then run:

go generate ./...

Thanks.

It works with tabby and Terminal.

It fails with iTerm2: https://gitlab.com/gnachman/iterm2/-/issues/10400#note_945236932

Btw, I fork zenity to https://github.com/trzsz/zenity , and I will replace the snippet later.

ncruces commented 2 years ago

That's precisely why I've resisted doing this so far:

Having said that, zenity proper takes an --attach flag.

I've wondered if I should add an Attach Option:

lonnywong commented 2 years ago

A simple option would be easy to use.

Can you get the ids inside the zenity? I want to attach to the parent process which having gui.

ncruces commented 2 years ago

In terms of the Go API, I agree that it'd probably be nicer to have AttachToParent() (and/or AttachToPid(pid)), and have the package figure out the correct app/window id for all platforms.

But I'm not sure I could do that, in particular, an AttachToParent() that finds the closest parent process with a GUI.

lonnywong commented 2 years ago

Will the GUI process load some special so / dll, or open some special devices?

lonnywong commented 2 years ago

On macOS, the GUI process is always the child process of the pid 1 process.

On Linux, the GUI process is the child process of the systemd process if it's not opened in a shell.

On Windows, https://stackoverflow.com/questions/3785698/how-can-i-tell-if-a-process-has-a-graphical-interface

ncruces commented 2 years ago

If you can live with an additional dock icon, this is the way:

ObjC.import('Cocoa')
var app = $.NSApplication.sharedApplication
var img = $.NSImage.alloc.initWithContentsOfFile('/path/to/icon.png')
app.setActivationPolicy($.NSApplicationActivationPolicyRegular)
app.setApplicationIconImage(img)

var app = Application.currentApplication()
app.includeStandardAdditions = true
app.activate()
ncruces commented 2 years ago

On both of the other platforms (windows, "linux") all dialogs create a taskbar icon. zenity allows this icon to be customised with the --window-icon flag.

So I'm leaning towards adding a zenity.WindowIcon(icon) option to pick this icon. On macOS, if you pass this option, I'll add a dock icon with the above code, which effectively solves the issue.

In addition, I may add a zenity.Attach(id) option mirroring the zenity flag --attach. In that case, instead of creating a new icon, dialogs will be attached to the application.

But I won't do this automatically, because a given application may not support showing a certain dialog (and some dialogs are actually impossible to launch in the context of another app).

lonnywong commented 2 years ago

Thanks.

My tool is running in terminal. Attaching to the terminal process is much better.

Btw, the GitHub of my tool: https://github.com/trzsz/trzsz-go

ncruces commented 2 years ago

zenity.WindowIcon(icon) is in master solving all focus issues.

I'll have a go at zenity.Attach(id) where id can be:

It'll be the app's responsibility to figure out what to attach to (the zenity command might try to attach to a parent terminal).

lonnywong commented 2 years ago

zenity.WindowIcon(icon) is in master solving all focus issues.

Thanks. It's great.

Will you add an attach option for zenity command? e.g.: zenity --progress --attach iTerm2. I'm using zenity --progress in https://github.com/trzsz/trzsz/blob/main/trzsz-iterm2/trzsz/iterm2/zenity_progress.py#L45 It would be better if the zenity progress dialog is a modal dialog.

I agree that it is the app's responsibility to figure out what to attach to. Thezenity.Attach(id) is a great option for advanced developers.

If the zenity command might try to attach to a parent terminal, will there be a zenity.AutoAttach() option?

ncruces commented 2 years ago

Yes. zenity --attach is already in head for macOS. It doesn't work for the progress and calendar dialogs, though. Not sure if those two can be made to work.

The problem with AutoAttach() is that it's an heuristic can of worms.

Attach() doesn't work very reliably. It works differently on different platforms and dialogs, attaching to different apps. Just on macOS, some dialogs can't attach, and even for those that can, it may not be possible to attach some apps (e.g. Safari or a particular terminal). And I still haven't done windows.

On linux/unix, AutoAttach() will be particularly hard to implement. It requires walking the process tree, for which there is no API in the standard library, and it requires mapping process ids to windows ids, for which suitable tools might not be installed.

So I my plan is really only to implement it for zenity, and only for macOS, where dialogs have focus issues, as a best effort to solve those focus issues. Note that dialogs are not modal or attached by default in the unix/linux zenity.

I'm happy to review PRs that lift that code and make it more general for other platforms/apps.

ncruces commented 2 years ago

zenity --attach/zenity.Attach(id) should be working as intended on all platforms, if you build from head.

On macOS attach implies modal. Some dialogs might not work, but those are tough/impossible to fix. On windows attach implies modal for some dialogs, not others. Please test.

lonnywong commented 2 years ago

Thanks a million. I will test it at the weekend.

lonnywong commented 2 years ago

This works for me on Windows.

package main

import (
    "fmt"
    "golang.org/x/sys/windows"
)

var user32 = windows.NewLazyDLL("user32.dll")

func getForegroundWindow() uintptr {
    hwnd, _, _ := user32.NewProc("GetForegroundWindow").Call()
    return hwnd
}

func main() {
    hwnd := getForegroundWindow()
    fmt.Println(hwnd)
}
lonnywong commented 2 years ago

This works on macOS. But I don't want to depend on gopsutil.

I think you did it better at https://github.com/ncruces/zenity/commit/b3fe9d02e904e9658da05758a21b3aae101bbb6e .

package main

import (
    "fmt"
    "os"

    "github.com/shirou/gopsutil/v3/process"
)

func main() {
    var pid int32
    ppid := int32(os.Getppid())
    for ppid != 1 {
        p, err := process.NewProcess(ppid)
        if err != nil {
            fmt.Println(err)
            return
        }
        pid = ppid
        ppid, err = p.Ppid()
        if err != nil {
            fmt.Println(err)
            return
        }
    }
    fmt.Println(pid)
}
ncruces commented 2 years ago

Check: https://github.com/ncruces/zenity/blob/b3fe9d02e904e9658da05758a21b3aae101bbb6e/internal/zenutil/window_darwin.go#L19-L52

Although, again, I'm not sure if this works for every "host" app on macOS.

What you're proposing for windows does not have similar semantics. You're assuming the foreground window belongs to a parent of the current process, which it might not. But this can be solved.

On unix the semantics are more complicated. Walking the process tree is easy: https://github.com/ncruces/zenity/blob/b3fe9d02e904e9658da05758a21b3aae101bbb6e/internal/zenutil/window_unix.go#L20-L42

Finding which of those processes has a window, if any, is harder. Notably because it requires tools to be installed which might not be installed.

lonnywong commented 2 years ago

How about this? It works on my macOS.

import "golang.org/x/sys/unix"

func GetParentWindowId() int {
    pid := os.Getppid()
    for {
        kinfo, err := unix.SysctlKinfoProc("kern.proc.pid", pid)
        if err != nil {
            return 0
        }
        switch kinfo.Eproc.Ppid {
        case 0:
            return 0
        case 1:
            return pid
        default:
            pid = int(kinfo.Eproc.Ppid)
        }
    }
}
lonnywong commented 2 years ago

On Ubuntu, It's not working with any window id.

$ xwininfo -root -tree

xwininfo: Window id: 0x2bc (the root window) (has no name)

  Root window id: 0x2bc (the root window) (has no name)
  Parent window id: 0x0 (none)
     30 children:
     0x80000a "gnome-shell": ("gnome-shell" "Gnome-shell")  1x1+-200+-200  +-200+-200
        1 child:
        0x80000b (has no name): ()  1x1+-1+-1  +-201+-201
     0x1400003 "UbuntuVM": ("tabby" "tabby")  1327x1181+1855+573  +1855+573
     0xa0002b "vmware-user": ("vmware-user" "Vmware-user")  200x200+74+27  +74+27
        1 child:
        0xa0002c (has no name): ()  1x1+-1+-1  +73+26
     0x1400009 (has no name): ()  1x1+0+0  +0+0
     0x1600003 "tabby": ("tabby" "Tabby")  200x200+0+0  +0+0
        1 child:
        0x1600004 (has no name): ()  1x1+-1+-1  +-1+-1
     0x1600001 "tabby": ("tabby" "Tabby")  10x10+10+10  +10+10
     0x1400000 "Chromium clipboard": ()  10x10+-100+-100  +-100+-100
     0xa00016 (has no name): ()  1x1+-1+-1  +-1+-1
     0xa00011 "vmware-user": ("vmware-user" "Vmware-user")  62x62+866+487  +866+487
        1 child:
        0xa00012 (has no name): ()  1x1+-1+-1  +865+486
     0xa0000e "vmware-user": ()  10x10+-100+-100  +-100+-100
     0xa0000a "vmware-user": ("vmware-user" "Vmware-user")  200x200+74+27  +74+27
        1 child:
        0xa0000b (has no name): ()  1x1+-1+-1  +73+26
     0xa00006 "vmware-user": ("vmware-user" "Vmware-user")  200x200+74+27  +74+27
        1 child:
        0xa00007 (has no name): ()  1x1+-1+-1  +73+26
     0xa00002 "vmware-user": ()  10x10+-100+-100  +-100+-100
     0x80001a (has no name): ()  1x1+-1+-1  +-1+-1
     0xa00001 "vmware-user": ("vmware-user" "Vmware-user")  10x10+10+10  +10+10
     0x800016 (has no name): ()  1x1+-100+-100  +-100+-100
     0x800012 "gnome-shell": ()  10x10+-100+-100  +-100+-100
     0x800011 (has no name): ()  1x1+-1+-1  +-1+-1
     0x800010 (has no name): ()  1x1+-1+-1  +-1+-1
     0x80000f (has no name): ()  1x1+-100+-100  +-100+-100
     0x80000e (has no name): ()  1x1+-1+-1  +-1+-1
     0x800008 (has no name): ()  1x1+-100+-100  +-100+-100
     0x800007 (has no name): ()  1x1+-100+-100  +-100+-100
     0x800006 "GNOME Shell": ()  1x1+-100+-100  +-100+-100
     0x800001 "gnome-shell": ("gnome-shell" "Gnome-shell")  10x10+10+10  +10+10
     0x600003 "ibus-xim": ()  1x1+0+0  +0+0
        1 child:
        0x600004 (has no name): ()  1x1+-1+-1  +-1+-1
     0x600001 "ibus-x11": ("ibus-x11" "Ibus-x11")  10x10+10+10  +10+10
     0x200002 (has no name): ()  10x10+0+0  +0+0
     0x200001 "gsd-xsettings": ("gsd-xsettings" "Gsd-xsettings")  10x10+10+10  +10+10
     0x800015 "mutter guard window": ()  3838x1931+0+0  +0+0
zenity --file-selection --modal --attach 0x01400003
ncruces commented 2 years ago

This is working for me, on Linux.

Clone master and do:

cd cmd/zenity
go run -tags dev . --error --modal

Should auto attach to the terminal, hopefully (does for me).

PS: file-selection dialog does not support attach or modal for some reason. Maybe that's why it didn't work for you.

lonnywong commented 2 years ago
OS zenity --file-selection zenity --error
Ubuntu 20.04 ✔️
Ubuntu 22.04

Could I call the zenutil.GetParentWindowId?

use of internal package github.com/ncruces/zenity/internal/zenutil not allowed
ncruces commented 2 years ago

Right, it's an internal API. I still haven't decided on a public one.

What doesn't work on 22.04? My package's auto-detection, or zenity itself?

lonnywong commented 2 years ago

What doesn't work on 22.04? My package's auto-detection, or zenity itself?

auto-detection returns 0 on 22.04.

$ xprop -root
_MUTTER_SENTINEL(CARDINAL) = 0
_NET_ACTIVE_WINDOW(WINDOW): window id # 0x0
_NET_CLIENT_LIST_STACKING(WINDOW): window id # 
_NET_CLIENT_LIST(WINDOW): window id # 
_NET_WORKAREA(CARDINAL) = 74, 27, 3764, 1904, 74, 27, 3764, 1904
_GTK_WORKAREAS_D1(CARDINAL) = 74, 27, 3764, 1904
_GTK_WORKAREAS_D0(CARDINAL) = 74, 27, 3764, 1904
_NET_DESKTOP_NAMES(UTF8_STRING) = "Workspace 1", "Workspace 2"
_NET_SHOWING_DESKTOP(CARDINAL) = 0
_NET_NUMBER_OF_DESKTOPS(CARDINAL) = 2
_NET_DESKTOP_GEOMETRY(CARDINAL) = 3838, 1931
_NET_DESKTOP_VIEWPORT(CARDINAL) = 0, 0
_NET_SUPPORTING_WM_CHECK(WINDOW): window id # 0x800006
ncruces commented 2 years ago

Your window manager doesn't set _NET_CLIENT_LIST, apparently. 😔

lonnywong commented 2 years ago

@ncruces

It works. At least works well on Windows and macOS for me.

I think we should close the issue. Thanks again.

lonnywong commented 2 years ago
$ ps -ef | grep iTerm2
  501 54757     1   0 Wed11PM ??         0:00.03 /Users/lonnywong/Library/Application Support/iTerm2/iTermServer-3.5.20220605-nightly /Users/lonnywong/Library/Application Support/iTerm2/iterm2-daemon-1.socket
  501 75470     1   0  6:47AM ??         4:07.69 /Applications/iTerm 2.app/Contents/MacOS/iTerm2

The parent process of zenity is 54757, which gets an error:

Error: Application can't be found. (-2700)

Any ideas to fix it?

This happens after upgraded the iTerm2. Exiting all the iTerm2 windows doesn't help. It works again after quit the iTerm2 and restart it.

ncruces commented 2 years ago

No idea tbh. It's these kinds of incompatibilities that make me think twice about commiting to an API for this.

I'd like to, at a minimum, be able to figure out if pid is a GUI app, give up if it isn't. I guess the easiest way to do that will be to just run osascript.

ncruces commented 2 years ago

AutoAttach(), as I had predicted, is an heuristic can of worms. So I'm going to go ahead and release what is available.

This is what I use internally for the zenity command, but this is subject to change if it causes issues: https://github.com/ncruces/zenity/blob/6899d6b87c8de5d9acb3933d2905cd6e6e6d391b/cmd/zenity/main.go#L455-L457

Regardless, zenity.Attach(id) exists as does zenity.WindowIcon(icon). This is pretty much the best I can do to fix this.

lonnywong commented 2 years ago

@ncruces Great job! Thanks very much.

llanc commented 3 weeks ago

Wonderful discussion. The tool I was working on recently used zenity and encountered this problem, I found a solution from your discussion! By the way, I have been starred the trzsz project for a long time, and I am very excited to get solutions to the problems from the discussions between you two excellent project authors. Thank you both again!