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

a minimized window incorrectly reports itself as visible #210

Closed
PerBothner opened this issue Sep 24, 2021 · 42 comments
Closed

a minimized window incorrectly reports itself as visible #210

PerBothner opened this issue Sep 24, 2021 · 42 comments

Comments

@PerBothner
Copy link

After a window is minimized, document.hidden and document.visibilityState report respectively false and "visible". The window.is_visible function reports the same.

This seems to contradict this.

Possibly related, window.set_minimized(false) sems to have no effect. Instead, to unminimize a window you first have to hide it, which seems unfortunate:

    if req.method == "show" {
      window.set_visible(false); // needed if window is minimized
      window.set_visible(true);
    }

Electron does have a isMinimized method - but is not needed because show works on minimized windows.

This is on Fedora Linux 34 running Gnome/Wayland.

@amrbashir
Copy link
Member

amrbashir commented Sep 24, 2021

Minimizing a window is not the same as hiding it so document.hidden ans document.visibiltyState are correct.

window.set_minimzed(false) might not bring the window to the foreground so you probably have to combine it with window.set_focus(true).

The gtk docs says that a window manager can choose to ignore the unminimzing request but window.set_focus(true) should force it be on the current desktop unminimzed and focused.

@PerBothner
Copy link
Author

"Minimizing a window is not the same as hiding it so document.hidden ans document.visibiltyState are correct."

The former statement does not imply the latter. The link I gave explicitly states that a minimized window should be considered "hidden".

Electron does this IMO correctly, for what it's worth.

"window.set_focus(true) should force it be on the current desktop unminimzed and focused."

Doesn't work for me. I just get a notification that the windiow "is ready" but it isn't made visible. I tried variations of:

    if req.method == "show" {
      window.set_minimized(false);
      window.set_visible(true);
      window.set_focus();
    }

(Note set_focus doesn't take an argument.)

@wusyong wusyong transferred this issue from tauri-apps/wry Sep 24, 2021
@amrbashir
Copy link
Member

I guess we need to investigate why set_minimzed(false) and set_focus() doesn't work as expected but I don't think we can do anything about document.hidden or document.visiblityState, that's part of webkit2gtk which we don't control.

@amrbashir
Copy link
Member

just tested on Windows and set_minimized(false) and set_focus() combination work as expected, will have to test Linux some other time.

@amrbashir
Copy link
Member

amrbashir commented Dec 2, 2021

I also tested the combination with set_visible.
I hid the window using:

window.set_minimized(true); // this returns the focus to previously focused window on Windows
window.set_visible(false);

Then I showed the window like this.

window.set_visible(true);
window.set_minimized(false);
window.set_focus();

@PerBothner
Copy link
Author

A minor problem is that the transition from hidden to minimized doesn't work - the window stays hidden (rather than minimized).

A bigger problem is getting toggle-minimize to work. This is needed for a "pop-up terminal" or "quake mode": One keypress minimizes the window, and a second one brings it back. For that I need to know whether the window is minimized. If I use a custom-titlebar I can intercept the "minimize" button, and in that case hopefully keep track of whether a window is minimized. But I don't know how to do that if using the system titlebar, since I can't detect if the user presses the Minimize button. Regardless, it would be more robust if webkit correctly implemented the page visibility api.

@amrbashir
Copy link
Member

detecting whether the window is minimized or not could be easily added to our api as is_minimized() option.

A minor problem is that the transition from hidden to minimized doesn't work - the window stays hidden (rather than minimized).

yeah that's what the code snippet I added is supposed to do. I wrote the snippet with apps like rofi in mind, where you open a window, then hide it completely but this is irrelevant to the issue anyway.

A bigger problem is getting toggle-minimize to work. This is needed for a "pop-up terminal" or "quake mode": One keypress minimizes the window, and a second one brings it back. For that I need to know whether the window is minimized. If I use a custom-titlebar I can intercept the "minimize" button, and in that case hopefully keep track of whether a window is minimized. But I don't know how to do that if using the system titlebar, since I can't detect if the user presses the Minimize button. Regardless, it would be more robust if webkit correctly implemented the page visibility api.

We can add is_minimized() method that can help you with that.

@PerBothner
Copy link
Author

A is_minimized function method would be very helpful and appreciated.

Doing the following will hide the window, not minimize it (on Linux/GtkWebKit):

window.set_minimized(true); 
window.set_visible(false);

If I leve off the set_visible, it is correctly minimized, but to show it I have to add a bogus set_visible(false). The following more-or-less works:

    if req.method == "minimize" {
      window.set_minimized(true);
    }
    if req.method == "hide" {
      window.set_minimized(true);
      window.set_visible(false);
    }
    if req.method == "show" {
      window.set_visible(false);
      window.set_visible(true);
      window.set_minimized(false);
      window.set_focus();
    }

Howeverhide followed by minimize doesn't work (as mentioned above).

If I had is_minimized I could probably do:

    if req.method == "show" {
      if window.is_minimized() {
        window.set_visible(false);
     }
      window.set_visible(true);
      window.set_minimized(false);
      window.set_focus();
    }

Still a bit clumsy, but not as bad.

@amrbashir
Copy link
Member

amrbashir commented Dec 3, 2021

Okay I have booted up my Linux and tried, set_minimized(false) and it works perfectly and as expected, with set_visible() or without it. Only issue I found is set_focus() is not working properly which I will try to fix soon.

A is_minimized function method would be very helpful and appreciated.

We are currently in feature-freeze phase and this won't be included in the upcoming v1 release of tao/wry/tauri, it will have to wait until v1 is out then I will implement it in the next branch which will be staging for v2.

With that said, I am closing this issue. is_minimzed() will be tracked in #257

@PerBothner
Copy link
Author

"Okay I have booted up my Linux and tried, set_minimized(false) and it works perfectly and as expected"

What is the sequence of calls you use to minimize and then un-minimize? I find I have to set_visible(false) in the show handler. If I call set_visible(false) in the minimize handler, then the window is hidden, not minimized. (Under Gnome on my Fedora 34 laptop, I can see the difference by clicking the [Window] "expose" key, which displays minimized but not hidden windows.) If I don't call set_visible(false) in the show handler, the window remains minimized.

@amrbashir
Copy link
Member

amrbashir commented Dec 3, 2021

My first test was set_minimized() only

let mut minimized = false;

if req.method == "toggleMinimize" {
  minimized = !minimized;
  window.set_minimized(minimized);
}

My second test was combining it with set_visible()

let mut minimized = false;

if req.method == "toggleMinimizeHidden" {
  if window.is_visible() {
    window.set_minimizd(true);
    window.set_visible(false);
  } else {
    window.set_visible(false);
    window.set_minmized(true);
  }
}

Keep in mind all requests are made from another window.

@PerBothner
Copy link
Author

Not sure what you're saying. None of these tests (ignoring typos) work, at least in my environment. I.e. switch from visible to minimized (not hidden) and back again,

"Keep in mind all requests are made from another window."

Yes. I use commands like this (which I have working fine when using an Electron window):

$ domterm --window=1 minimize
$ domterm --window=1 show

@amrbashir
Copy link
Member

amrbashir commented Dec 3, 2021

Please post your repo or at least a compiled version of your app, I can try it later.

@PerBothner
Copy link
Author

By the way: document.hidden seems to be working correctly now.

I can now send one of the requests to minimize/hide/show/toggle-minimize/toggle-hide to a DomTerm window, and it behaves as expected. However, the code needed for it to work is rather kludgy:

        // The following logic for minimize/hide/show is a bit weird,
        // but seems to what is needed to get things to work correctly.
        if req == "minimize" {
            if ! window.is_visible() {
                if window.is_minimized() {
                    window.set_visible(false);
                    window.set_minimized(false);
                }
                window.set_visible(true);
            }
            window.set_minimized(true);
        } else if req == "hide" {
            window.set_visible(false);
            if window.is_minimized() {
                window.set_minimized(false);
            }
        } else if req == "show" {
            if window.is_minimized() {
                window.set_visible(false);
                window.set_minimized(false);
            }
            window.set_visible(true);
            window.set_focus();
        }

I.e. to transition from hidden to minimized (in a way thar can deal with a future show) we need to do:

window.set_visible(false);
window.set_minimized(false);
window.set_visible(true);
window.set_minimized(true);

These redundant transitions seem to be what is necessary. I don't know why. It works - but it sure is ugly.

@amrbashir
Copy link
Member

your code could be simplified to just this.

        march req {
        	"minimize" => {
            	window.set_minimized(true);
            }
        	"hide" => {
        		window.set_minimized(true);
            	window.set_visible(false);
        	}
        	"show" => {
            	window.set_visible(true);
            	window.set_minimized(false);           
            	window.set_focus();
        	}
        }

@PerBothner
Copy link
Author

your code could be simplified to just this. ...

Yes, that would be more elegant - but it doesn't work. For example minimize followed by show fails to make the window visible.

@amrbashir
Copy link
Member

amrbashir commented Feb 6, 2023

on what os? this should be working on Windows and macOS, linux might be weird because it is dependent on the window manager in use.

@PerBothner
Copy link
Author

Fedora 37, running X11-Gnome (not Wayland at the moment). Wry 0.26.0.

@PerBothner
Copy link
Author

Note a more complex transition: normal -> hide -> minimize -> show. It took me a while to get that working.

@amrbashir
Copy link
Member

fails to make the window visible.

by this what do you mean exactly? is the window not present in the dock? or is the window visible but doesn't get brought to the front and focused?

@PerBothner
Copy link
Author

by this what do you mean exactly? is the window not present in the dock? or is the window visible but doesn't get brought to the front and focused?

There is no dock in my setup. The difference between hidden and minimized is only apparent when I request "Expose Windows" (pressing the Window key): A minimized windows shows up on Expose (and can be made visible by clicking on it), while a hidden window does not.

When I minimize a window, it is no longer visible - but it becomes temporarily visible when I Expose by pressing the Super key (and can be made permanent if I click on the exposed window).

However, minimize followed by show gives me a brief popup notication that the window is "ready" - but it is not actually made visible.

@amrbashir
Copy link
Member

amrbashir commented Feb 6, 2023

@FabianLars could you test this on your gnome when you have a chance? last time I checked this, it worked fine on AwesomeWM

@FabianLars
Copy link
Member

FabianLars commented Feb 7, 2023

tbh i'm super confused what i'm supposed to test here.
on wslg ubuntu (gnome-ish):

  • minimizing the window is not enough to make document.hidden == true (i think i agree that it should btw)
  • minimizing the window, and then calling show, without calling unminize does not bring back the window -> works as intended right?
  • appWindow.hide() sets document.hidden to true. show() does get it back fine.
  • if i called .hide() on a minimized window, then call show() it will be visibly on the screen again, ignoring the minimized state

what else should i test?
also i can test on an actual linux installation later today too.

@amrbashir
Copy link
Member

amrbashir commented Feb 7, 2023

I am mainly interested in this snippet using tao or wry:

        march req {
        	"minimize" => {
            	window.set_minimized(true);
            }
        	"hide" => {
        		window.set_minimized(true);
            	window.set_visible(false);
        	}
        	"show" => {
            	window.set_visible(true);
            	window.set_minimized(false);           
            	window.set_focus();
        	}
        }

try triggering the req minimize then show and see if the window gets unminimized and brought to the foreground

@FabianLars
Copy link
Member

Nope, the window stays minimized using wry's (and therefore tao's) latest git commit. Same if i cut it down to just window.set_minimized(true); & window.set_minimized(false); btw
(p.s. note that tauri's unminimize function in v1.2 worked fine as far as i can tell)

@amrbashir
Copy link
Member

that's weird, the minimize code hasn't been touched since the tauri beta or even from before that.

@FabianLars
Copy link
Member

using wry@0.23 (what tauri 1.2 uses) doesn't make it work either so either i'm doing something really stupid, or tauri does additional stuff on unminimize

@amrbashir
Copy link
Member

usually it needs to be paired with window.set_focus

@FabianLars
Copy link
Member

okay i tested unminimize in tauri again and now it doesn't work either. (not with setFocus either). I'll switch to my actual linux installation, i don't trust wslg enough.

@FabianLars
Copy link
Member

FabianLars commented Feb 7, 2023

Okay so, using fedora 37:

  • kde + x11 = set_minimize(false) works (this at least shows that i'm probably testing it correctly)
  • kde + wayland = broken
  • gnome + x11 = broken
  • gnome + wayland = broken
    note that i added the gnome installation ontop of the fedora kde spin so maybe something broke, but it matches PerBothner's observations on gnome

@PerBothner
Copy link
Author

document.hidden does correctly return true after a minimize on Fedora 37, Gnome-X11, wry: 0.26.0, webkitgtk: 2.38.3, appleWebKit: 605.1.15.

@amrbashir
Copy link
Member

Just tested on KDE using window_debug example in this repo, and it works fine so I think this is another Gnome bug that we have no control over.

@FabianLars
Copy link
Member

did you also test kde x11 vs kde wayland? (on most distros you can switch it after logging out, there should be some gear icon or something on the login screen)

@amrbashir
Copy link
Member

I am on KDE x11

@amrbashir
Copy link
Member

wouldn't be surprised if it doesn't work on Wayland, because wayland is more restrictive and we can't fix that either.

@FabianLars
Copy link
Member

yeah, didn't work for me on kde wayland, only on kde x11. Though i could swear it used to work better-ish some months ago but probably just wishful thinking.

but tbh, it feels pretty bad if it only works on kde x11, considering the popularity of gnome and gnome based DEs, and the rise of wayland (default on fedora and kubuntu (at least on ubuntu, not actually sure about their kde version)).

Maybe a bit too philosophical, but can we really consider an API working on Linux if it doesn't work as expected for most Linux users even if it's technically not our fault?
Or well, maybe it's something that should have a workaround in tauri, not the low-level windowing library!

@amrbashir
Copy link
Member

There is one last thing I want to test so I will install Gnome to investigate a bit further but no promises.

@PerBothner
Copy link
Author

I was going to say "but it works using Electron or Qt" ... except it no longer does. Both Electron and Qt the normal -> hidden -> minimize sequence make the window visible, and I'm fairly sure it didn't used to do that. (Though in the past I mostly tested on Wayland.) FWIW things work on fine on macOS.

I'll try to switch to Wayland later today to check that is also broken there.

I suspect the regression on Gnome on some relatively-new regression, and it would be best to report it. (For what it's worth, I did report a bug on webkitgtk - and it got fixed and the fix is finally in Fedora.)

@amrbashir
Copy link
Member

I was going to say "but it works using Electron or Qt" ... except it no longer does.

Which I find weird too since we are simply calling GTK APIs directly so you'd expect it to work on Gnome without a problem, in fact, I was always worried about other Window Managers but nowadays I have seen a lot of APIs break on Gnome that I am quite sure used to work well.

@amrbashir
Copy link
Member

Yup just tested on Gnome and it doesn't work. Unfortunately I don't think there is much we can do since they state in their docs that the window manager could choose not to unminimize the window see https://docs.gtk.org/gtk3/method.Window.deiconify.html

@PerBothner
Copy link
Author

they state in their docs that the window manager could choose not to unminimize the window

No, that's not how I read it. It says you can't count on it being deiconified because ther user or window manager could re-iconify it. If the request to deiconify is overridden/ignored by the WM that would definitely be contrary to expected WM behavior - and (for a general-purpose WM with sane defaults) a bug. The point is the Gtk is at the mercy of the (buggy) WM.

@amrbashir
Copy link
Member

amrbashir commented Feb 9, 2023

No, that's not how I read it. It says you can't count on it being deiconified because ther user or window manager could re-iconify it.

Actually, the window manager could still choose to ignore the de-iconification request.

The point is the Gtk is at the mercy of the (buggy) WM.

Not just GTK but raw x11 APIs too, the window manager could simply choose not to do a request, it is pretty common thing actually.

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

No branches or pull requests

3 participants