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

Windows positioned off-screen if hosting application has no visible windows #46

Closed
kenjiuno opened this issue Jan 21, 2015 · 8 comments
Closed

Comments

@kenjiuno
Copy link
Contributor

UpdateDialog goes out of screen, if no app window available.

P.S. My app has no main window. It stays at tray icon!

Here is the previous behavior I have ever seen.

olderver

UpdateDialog is located around (34, 22) which will be default behavior of wxTOPLEVEL_EX_DIALOG flag.

Applying commits by #22 issue.

outside

UpdateDialog is moved beyond top left corner.

I have modified CenterWindowOnHostApplication function so that AskPermissionDialog won't get out of screen.

But subsequent UpdateDialog gets out.
Because CenterWindowOnHostApplication function will simply do centering.
UpdateDialog will keep center position against my process's window AskPermissionDialog:

outside2

So I have tweaked around CenterWindowOnHostApplication function. UpdateDialog won't get out now.

moveinside

I have my experimental code for this issue.

However I want to discuss: centering is ideal for usual cases?

In my case, I'll disable CenterWindowOnHostApplication and, use it for my customer.
I have worked hard today for this issue, but I began to think whether centering is ideal or not?

@vslavik
Copy link
Owner

vslavik commented Jan 21, 2015

UpdateDialog goes out of screen, if no app window available.

That's undeniably a bug.

I have modified CenterWindowOnHostApplication function so that AskPermissionDialog won't get out of screen.

So where's the PR? And does it work in multi-display case?

UpdateDialog will keep center position against my process's window AskPermissionDialog:

Sorry, but this begs a question: what the heck are you doing? I appreciate that you previously found a problem in this corner case, but let's make one thing clear: this is not normal. It seems you are intentionally making both of these windows appear (not having the user do it explicitly)? Why? There's a reason why WinSparkle asks for permission first: you shouldn't do the checks if the user disagrees. And the above screenshot is pretty blatant in-your-face "I'll ignore your answer anyway"...

I have worked hard today for this issue, but I began to think whether centering is ideal or not?

Perhaps you should actually spend some time arguing or at least explaining your position, disagreement or whatever it is you're trying to communicate. Because right now all I can tell you is that well, obviously I considered it a good idea (see also #22, #34, #37). So?

@vslavik vslavik changed the title Centering is ideal for usual cases? Windows positioned off-screen if hosting application has no visible windows Jan 21, 2015
@kenjiuno
Copy link
Contributor Author

Hi.

I'm sorry for confusing so much... I have no intention to offend you.

blatant in-your-face "I'll ignore your answer anyway"...

Sorry. I mean I have limited period to fix this issue for my customer. So, I'll simply disable it for now.

I'll try to provide or consider incoming ideas in the future.

So where's the PR? And does it work in multi-display case?

I'll push a pr in the future. It won't consider multi-display case. I need more research for other cases which is not covering this issue.

There's a reason why WinSparkle asks for permission first

Yes, its part include 2 problems:

  • UpdateDialog extends vertically if there is an available update. It causes get out of screen probably.
  • AskPermission can be treated as your app main window. If your main app window is small and located at top-left corner or bottom-right corner... It will get out.

kenji uno

@vslavik
Copy link
Owner

vslavik commented Jan 21, 2015

I'm sorry for confusing so much... I have no intention to offend you.

blatant in-your-face "I'll ignore your answer anyway"…
Sorry.

No, you misunderstand. You are doing this to your users. You are asking them for permission to do something and without waiting for their answer, you do it anyway. That's terrible.

Again, why are you doing this? And also, how are you doing this? Because this is not normal WinSparkle behavior. It seems to be that you are calling win_sparkle_check_update_with_ui() without user interaction to trigger it — don't do that.

It won't consider multi-display case.

The point was that any code included in WinSparkle absolutely has to… I'll have a look myself, then.

@kenjiuno
Copy link
Contributor Author

Hmm.

I admit that my one is very rare case.

But I want to believe it is not an abnormal case... My app has no main window. It stays at tray icon.

Here is my pseudo code using WinSparkle.

            try {
                WinSparkle.win_sparkle_set_appcast_url(AppcastUrl);
                ...
                WinSparkle.win_sparkle_init();
                if (auto)
                    WinSparkle.win_sparkle_check_update_without_ui();
                else
                    WinSparkle.win_sparkle_check_update_with_ui();
            }
            catch (BadImageFormatException) { }

win_sparkle_init will show AskForPermission, if user has never answered.

auto is True, and then call win_sparkle_check_update_without_ui, because the code is called from startup code. It may display UpdateDialog, if update is avail.

@kenjiuno
Copy link
Contributor Author

I'm sorry.

Is the main problem that I didn't mention "My app has no main window. It stays at tray icon." at first post?

@vslavik
Copy link
Owner

vslavik commented Jan 21, 2015

This code is bad:

  1. You should never call win_sparkle_check_update_with_ui() automatically - only call it if the user explicitly asks to check for updates.
  2. You're just duplicating what win_sparkle_init() does.

This is what you should do instead:

win_sparkle_set_appcast_url(AppcastUrl);
win_sparkle_set_automatic_check_for_updates(auto);
win_sparkle_init();

That's the clean way to do what you apparently intended to do.

P.S. All this is explained in winsparkle.h documentation…
P.P.S. For crying out loud, don't silently catch & trash serious exceptions like that!

@kenjiuno
Copy link
Contributor Author

Hi.

Thanks for code advising. I'll try it. And I'll check docs written at winsparkle.h later.

don't silently catch & trash serious exceptions like that!

Oh, I have forgotten about that catch for long time... Ok, I'll check it too.

vslavik added a commit that referenced this issue Jan 21, 2015
Make sure that WinSparkle's windows are always fully visible. Not only
when positioning on the host application, but also the window size
changes (e.g. transitioning from "checking for updates" to "updates
available").

See #46
@kenjiuno
Copy link
Contributor Author

Hi.

Thank you for fixing of this issue. I have tested the latest c90ddf3, and there is no problem in my case now.

I have erased win_sparkle_check_update_xxx call at startup, and there is only one window (AskPermission or UpdateDialog) simultaneously now.

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

No branches or pull requests

2 participants