Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a powershell script to manage focus on Windows #5

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

kaste
Copy link
Contributor

@kaste kaste commented Mar 12, 2023

Fixes #4

Revert cd1a2d5 as Sublime Text's bring_to_front politely asks for the focus which is usually not enough when a browser has input focus.

Since we have a builtin method for MacOs in place just implement a powerscript equivalent for Windows.

Unfortunately Linux then still needs a tool ("xdotool").

Copy link
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for me! Thank you for looking into it.

Does the Windows version work now?

It's disappointing (and honestly a bit strange) that bring_to_front does not bring to front. I'd love to use the native API. Does this thread contain anything useful? sublimehq/sublime_text#444

A comment mentions:

other_window.focus_view(other_window.views()[0])
other_window.bring_to_front()

@kaste kaste force-pushed the new-focus-approach branch 2 times, most recently from dd39621 to 9088caf Compare March 13, 2023 08:27
Comment on lines 15 to 16
if NATIVE_FOCUS_WINDOW:
self.window.bring_to_front()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way I'd probably use feature detection rather than version sniffing. Something like:

Suggested change
if NATIVE_FOCUS_WINDOW:
self.window.bring_to_front()
if hasattr(self.window, 'bring_to_front'):
self.window.bring_to_front()

In JS I'd just write self.window.bring_to_front?.() but it doesn't look like Python has that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, beacuse the (possible) environments in Web development are basically
hostile and complex.

Here in ST land, we would rather tell Package Control a ST3 version of
this plugin, and then go on with ST4 without version sniffing, IMO. Likely
that ST3 version would be final too as the code is stable since years, well
before I came at least.

Fixes GhostText#4

Sublime Text's `bring_to_front` politely asks for the focus which is
often not enough when a browser has input focus.

But a) if it does it is instant and b) it helps when all windows are
minimized.

As we cannot check if `bring_to_front` actually did what we want, use
the scripts we always had here too, unconditionally.

For Windows, replace the used method with a powershell script which
works on newer Windows versions without installing the rather
idiosyncratic "nircmd".

Unfortunately Linux still needs a tool ("xdotool").
@kaste
Copy link
Contributor Author

kaste commented Mar 13, 2023

I did restore the bring_to_front call and then (unconditionally) we also
do the script dances we always did.

This is because at least the powershell script doesn't work when Sublime only
has minimized windows. (Why do I want to pluralize windows to windowses.
Always, 🙄).

As for

other_window.focus_view(other_window.views()[0])
other_window.bring_to_front()

this does not help because the OS prevents stealing the focus when a browser
has input focus (e.g. being in a textarea). Using the normal, polite method.
We're using force and we cannot ask ST to do the force for us. This violates
app UX standards.

We only do this because we enhance the editing of the textarea element. We're
like an accessibility tool here, basically an input helper. As such we were
allowed to use Windows accessibility API's to do this "native" without calling
out to a script. But then we had to do this for Mac and Linux as well.

@fregante
Copy link
Member

this does not help because the OS prevents stealing the focus when a browser
has input focus

I tried that and it doesn't seem to be right (at least in macOS and Safari). I tried:

  1. Open https://ghosttext.fregante.com/troubleshooting/
  2. Enable GhostText

Note: There's one field on the page, the user does not need to select it

@fregante
Copy link
Member

Anyway it looks good, it can be merged if ready

@kaste
Copy link
Contributor Author

kaste commented Mar 13, 2023

For Windows there are several rules https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow#remarks

If I click on an icon in the browser that is also a clear interaction with that
window. From my perspective, I would prevent focus stealing here as well, I think.

I just don't know why it sometimes works. From that I get the feeling we just need to call
everything in the right order and right timing so that Windows thinks: yeah the user wants
to shift to Sublime, let's allow that.

The simple powerscript approach isn't 100% though, and I hate that. If it doen't work often
enough I probably annoy you with another PR. I still think nircmd is a not an option because it is an totally unusual, unknown third-party.

@fregante fregante merged commit 6a26540 into GhostText:main Mar 13, 2023
@fregante
Copy link
Member

I think there are a few ways GhostText could go about this in the future, like setting a ghosttext:// URI that the editor could then register. The focus switch would then be handled by the OS and could theoretically even work on a mobile device.

The challenge there would be that not every editor can be set up to handle such a protocol, so it would be less flexible (I'm not a fan of that)

@fregante
Copy link
Member

By the way some input would be welcome here if you have any opinions:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants