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

Moving window to correct coordinates before maximizing #1765

Merged
merged 2 commits into from
Sep 13, 2021

Conversation

gchamon
Copy link

@gchamon gchamon commented May 28, 2020

Using issue #1689, specifically the one comment about setting height and width to anything less than 100% (#1689 (comment)) I managed to track down the condition that deals with maximized windows, but not fullscreen:

         # lines 265-267
         if width_percents == 100 and height_percents == 100:
             log.debug("MAXIMIZING MAIN WINDOW")
             window.maximize()

According to this answer on stackoverflow, you should move the window to the correct coordinates, in the correct display, before attempting to maximize the window. I just replicated this with the correct coordinates calculated beforehand and in my machine it seems to have fixed the issue.

EDIT:
seems like it should also fix:

#1761
#1745
#1738

@Davidy22
Copy link
Collaborator

Davidy22 commented Sep 8, 2021

Going to mark other potentially related issues so I can get back to them after this is merged: #1837 #1844 #1818 #1817.

Copy link
Collaborator

@Davidy22 Davidy22 left a comment

Choose a reason for hiding this comment

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

Hello, can't test your changes because I don't have a second monitor, but the change looks right and you say it works for you. I checked this line against our fixed CI and it passes, so we're basically ready to merge, it would be nice if you could fill in a release note file for us before that:

make reno SLUG=fix_move_window_to_correct_coordinates_before_maximising

in the top level of the project directory should pop a release notes template for you to fill in. Delete the sections that aren't relevant, fill in the ones that are and we'll be good to go.

@gchamon
Copy link
Author

gchamon commented Sep 8, 2021

I still have a second monitor, I can test my changes again, to see if this PR is still relevant.

@Davidy22 Davidy22 force-pushed the fix-fullscreen-display branch from d2e2e9c to 98e29e2 Compare September 10, 2021 20:20
@Davidy22
Copy link
Collaborator

Davidy22 commented Sep 10, 2021

Went and rebased and added the release notes file so we can see the build status of this for now.

EDIT: Build status looks good so when you or anyone affected can confirm this is still good I'll merge.

Copy link
Collaborator

@Davidy22 Davidy22 left a comment

Choose a reason for hiding this comment

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

I'll take the emote as confirmation that this still works for you and merge

@kpalczewski
Copy link

@Davidy22 I can't open guake for a user, but I can open it with sudo. I don't know if this matters.

I have set Guake Preferences -> Main Windows -> Height to half. Anything other than full height makes guake to improperly show up on a monitor with dock on right side: guake windows has width that is too wide - it should be shorter by a dock's width. Because of that it shows up on my left monitor (dock is on my right monitor).

If I press F11 it changes window size properly and moves to just 1 monitor.

Guake Version: 3.7.1.dev119

@Davidy22
Copy link
Collaborator

Davidy22 commented Sep 14, 2021

Oh alright, going to have to monkey a unity dock onto my not-ubuntu distro to test properly, and finally get me a second monitor. The opening just for a super user should just be happening when installing from source, been making some changes to clean up upgrading, #1897 should clean up the last of that one.

@Davidy22
Copy link
Collaborator

Actually @kpalczewski, there is #1729 which is in the queue, but has some issues that still need clearing up. To note, the contributor states that it is untested, and hasn't come back with further testing results, but it looks related to your issue. If that branch fixes the issue for you, then I can look at fixing the rest of that pull request and merging.

@kpalczewski
Copy link

@Davidy22 that version doesn't run anymore on Ubuntu 20.04
Current monitor geometry window_rect.x: 1920 window_rect.y: 0 window_rect.height: 1200 window_rect.width: 1920 Traceback (most recent call last): File "/usr/lib/python3/dist-packages/dbus/bus.py", line 177, in activate_name_owner return self.get_name_owner(bus_name) File "/usr/lib/python3/dist-packages/dbus/bus.py", line 361, in get_name_owner return self.call_blocking(BUS_DAEMON_NAME, BUS_DAEMON_PATH, File "/usr/lib/python3/dist-packages/dbus/connection.py", line 652, in call_blocking reply_message = self.send_message_with_reply_and_block( dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NameHasNoOwner: Could not get owner of name 'org.guake3.RemoteControl': no such name

later I can try to make changes from branch ubuntu-lts-hotfix onto latest guake version from github.

@Davidy22
Copy link
Collaborator

Davidy22 commented Sep 15, 2021

Eh, there were a lot of issues in there on visual inspection, it's possible that version never ran on ubuntu, was an outside hope that it would work as advertised.

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.

3 participants