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

Firefox subwindows pop up on the wrong monitor #59

Closed
wilhelmy opened this issue Dec 8, 2017 · 39 comments · Fixed by #303
Closed

Firefox subwindows pop up on the wrong monitor #59

wilhelmy opened this issue Dec 8, 2017 · 39 comments · Fixed by #303
Labels

Comments

@wilhelmy
Copy link
Collaborator

wilhelmy commented Dec 8, 2017

I have 3 monitors attached to my laptop: the internal screen, one to the left of it and one to the right.

When I keep firefox on the rightmost or center screen, some subwindows like context menus or other popup menus (e.g. built-in dropdowns or input suggestions on previously filled forms but not ones which are implemented in javascript, I assume because they're not handled directly by GTK) appear at the correct position, but on the leftmost screen.

If I keep firefox on the leftmost screen, the popup works as expected.

It also doesn't happen with thunderbird or other GTK2 or GTK3 software, but affects all versions of firefox I've used on this machine up until 57.

Any ideas? Can anyone confirm this issue as being notion specific and not a firefox bug?

@raboof
Copy link
Owner

raboof commented Dec 8, 2017

I suspect this might not strictly speaking be a notion bug, but a problem that is rare under other window managers.

It has been a while, but if I recall correctly the situation is like this: X11 has a 'window hierarchy'. Notion creates a 'window' in this hierarchy for each screen, and places client windows relative to that intermediate 'window'. Now when deciding where to place the popup, applications may calculate the position relative to this parent 'window', but then use these coordinates in a place where they're interpreted as relative to the X11 root window. Since most window managers don't have an intermediate 'screen', this is not obvious. It might be a little more subtle than this even (might be related to EWMH _NET_VIRTUAL_ROOTS?), I don't quite remember.

So this is not necessarily a notion bug, might even be a firefox bug (though rarely triggered) - probably in the grey area in between.

@knixeur
Copy link
Collaborator

knixeur commented Dec 9, 2017

I can confirm this happens to me too @wilhelmy , according to #57 's second comment, it seems it not only affects notion.

@md1023
Copy link

md1023 commented Feb 2, 2018

I've found out Seamonkey 2.49.1 opens browser popup window in a new Notion tab (which is good and expected), while Firefox 57 just goes into this link in same tab without opening Notion or browser tab.

@raboof
Copy link
Owner

raboof commented Feb 2, 2018

Even though this might not only affect Notion, it would still be good to fix of course :). Though perhaps it might be a combination of fixes on the Notion and Firefox side.

@md1023 What you describe sounds like a separate issue, right? Does it also happen on other window managers?

@md1023
Copy link

md1023 commented Feb 2, 2018

@raboof yes, separate one. I prefer Seamonkey so there's no issue to me when using NotionWM :-) I used Unity or LXDE day ago, both browsers behaved properly (e.g. both created new browser windows).

@raboof
Copy link
Owner

raboof commented Feb 2, 2018

@md1023 please open a separate issue then :)

@shokre
Copy link

shokre commented May 7, 2018

I can confirm this happens to me too. Mostly on Firefox, but sometimes in Chrome also.

After resizing the frame containing main window of an app, menus/popups appear in the right place.

@cuteboyworld
Copy link

i was able to fix this by switching back to Firefox 56
https://ftp.mozilla.org/pub/firefox/releases/56.0/linux-x86_64/en-US/
i don't know how or why but running firefox from that folder fixed everything (even the old ff install)

@esaule
Copy link

esaule commented Jun 13, 2018

I am also experiencing this problem. However old firefox won't solve it.
Is there a way to fix this issue or at least to work around it?

@cuteboyworld
Copy link

what i do is
LD_LIBRARY_PATH="PATH TO FIREFOX 57"
and run the firefox executable that inside that folder
and kill it immediately after (while its updating addons)
then i run my normal firefox
i haven't tested this with firefox quantum

@knixeur
Copy link
Collaborator

knixeur commented Sep 26, 2019

@raboof was pointing in the right direction, if you just remove _NET_VIRTUAL_ROOTS atom set in ioncore/netwm.c, so _NET_VIRTUAL_ROOTS is not set, it works correctly.
I dumped notion's and i3's root properties and i3 doesn't set it. It set other variables which I did set manually to test, but the only thing that affects Firefox is _NET_VIRTUAL_ROOTS

knixeur@a64250a

EDIT: Not so fast.. it's working if you have only one full frame, if it's splitted in half, it popups in the first (left) one.

EDIT2: Btw, i3 may not be a reparenting WM (I haven't checked) so it might not need to set it.

@knixeur
Copy link
Collaborator

knixeur commented Sep 26, 2019

Regarding the first edit, it's clearly not the real solution as the popup is off a little bit to the top, probably the same amount of the size of the title bar. So it's not receiving the correct offset anyway.

@shokre
Copy link

shokre commented Sep 26, 2019

@knixeur I have a feeling that you are close to solution.

Resizing a frame fixes pop-up positioning problem.
Also, moving WClientWin to different frame and back to original frame fixes the problem.

Maybe the bits you need are there.

@shokre
Copy link

shokre commented Sep 26, 2019

Lately I've been using JetBrains Toolbox , and noticed that on first start Toolbox window fills whole frame, but when moved to different frame, or if the containing frame was resized, Toolbox window reverts to WM_NORMAL_HINTS(WM_SIZE_HINTS) size.

@pigmej
Copy link
Contributor

pigmej commented Sep 30, 2019

@knixeur I've checked your patch. It made my second screen completely froze. Nothing was working on it (it seemed to be frozen basically). I could run "rofi" as it's kinda popup, but none of the other actions worked. Also when rofi was shown then it never disappeared.

Everything returned back to normal on master.

@knixeur
Copy link
Collaborator

knixeur commented Sep 30, 2019

@pigmej lol, sorry, there's actually a commit that shouldn't be there and will cause a lot of trouble, I'll remove it and push back to the branch.

Done, removed conflicting commit 😄
You might want to just remove virtual_roots from your local. In this branch there are some new properties being set, current_desktop specially is wrong and hardcoded, the other ones I'm not sure if they're correct (seems so). So far, they don't seem to affect other things.

@wilhelmy
Copy link
Collaborator Author

@knixeur I still have the lua script for current_desktop, maybe that would help?

@knixeur
Copy link
Collaborator

knixeur commented Sep 30, 2019

Sure, make a gist or maybe commit it inside contrib as an example. Anyways I'm starting to think this is more related to who's Firefox parent.

@molecular
Copy link

molecular commented Oct 28, 2019

@wilhelmy, thanks for consolidating tickets.

I just read through this one, wasn't aware of it.

@raboof wrote:

Now when deciding where to place the popup, applications may calculate the position relative to this parent 'window', but then use these coordinates in a place where they're interpreted as relative to the X11 root window.

This matches the symptoms pretty well.

gray area between firefox and X11/notion

yeah, probably. I doubt it would be fruitful (since this affects only a small group of people), but maybe we should open an issue with firefox?

@wilhelmy
Copy link
Collaborator Author

There's this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=61000
But I doubt anything will happen with it.

Either way, I've been seeing strange behaviour on multihead setups also with OpenSCAD lately, such as the menu position being off. Even if it is also a firefox bug, I've been thinking we can work around it in notion by just putting the virtual roots/reparenting information in the right place.

EWMH is a bit unclear on how all of this is supposed to work, maybe that's the reason for this confusion among application developers.

@molecular
Copy link

molecular commented Oct 29, 2019

interesting that you bring up OpenSCAD: I've also been experiencing issues with it: the rendered preview is sometimes frozen (I guess it's opengl): nothing updates when I zoom or pan or change code. My painful workaround is to restart X. Might be unrelated, though.

Now another issue appeared: I can't even see the editor pane any more, but I guess that's a different issue probably due to an update of OpenSCAD. Since I don't need it right now I gave up looking for that editor frame.

sorry for spamming likely unrelated issues here.

@molecular
Copy link

molecular commented Oct 29, 2019

haha LOL, from that mozilla ticket:

Opened 19 years ago

lmao.

@chreekat
Copy link

chreekat commented Nov 11, 2019

With this layout, the popups are basically always in the wrong place, regardless of which display Firefox is on:

image

That definitely suggests firefox is taking some relative offset and trying to paint to an absolute offset, which is often empty space in my case.

@wilhelmy
Copy link
Collaborator Author

Yes, I'm not sure it was mentioned above but Firefox will base its subwindows at an absolute (0,0). Perhaps this can be worked around by fixing the way windows are reparented in Notion, but we don't know for sure.

@raboof raboof added the bug label Dec 29, 2019
@raboof
Copy link
Owner

raboof commented Dec 29, 2019

(marking this 'bug', as even if it remains to be seen if this is 'strictly speaking our fault', it is annoying enough to have priority)

I checked this problem also exists on the latest notion3, so it's not a regression, so I guess it shouldn't hold up the Notion4 release.

@molecular
Copy link

so I guess it shouldn't hold up the Notion4 release.

Definitely not. I don't know anything about notion4 release, but I don't think it should be delayed in any way because of this issue, which is more of a firefox issue (19 years old!!) and it's not notion4s job to work around that imo.

@raboof
Copy link
Owner

raboof commented Jan 9, 2020

which is more of a firefox issue (19 years old!!)

Maybe I'll dig into it again at some point, be a good OSS citizen and fix the firefox issue :D. A reproducer with an application that is not so huge would be good though...

But indeed not before notion4, just a couple of tasks left (https://github.com/raboof/notion/projects/1)

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Jan 9, 2020

I also had problems with OpenSCAD's menu bar menus popping up on the wrong screen. This is Qt software however. Maybe we can actually fix this issue within notion. I still haven't completely given up on this but I'm busy doing other stuff right now unfortunately (well, the stuff is interesting at least but not at all notion related) and will also look into it once I have more control over how I spend my time again.

I really want to help 4.0 come along (or maybe 4.1) and this bug is definitely on my roadmap somewhere (not necessarily for 4.0, which I don't want to stall).

@raboof
Copy link
Owner

raboof commented Feb 1, 2020

Some more observations:

Let's keep this issue specifically about the right-click context menu, to avoid confusion ;).

You can easily reproduce the problem even on a machine with only one screen by setting the 'panning' of the screen, for example with something like xrandr --output eDP1 --panning 3840x2160+100+0 to show the popup slightly too far too the left.

When you open the popup near the left of the screen, it is still positioned too far to the left, but never so far to the left that it falls outside the screen. I have verified this with xtrace:

000:>:2c95: Event ConfigureNotify(22) event=0x07000033 window=0x07000033 above-sibling=0x08800001 x=100 y=278 width=210 height=220 border-width=0 override-redirect=true(0x01)
000:>:3047: Event ConfigureNotify(22) event=0x07000033 window=0x07000033 above-sibling=0x08800001 x=100 y=219 width=210 height=220 border-width=0 override-redirect=true(0x01)
000:>:33be: Event ConfigureNotify(22) event=0x07000033 window=0x07000033 above-sibling=0x08800001 x=122 y=197 width=210 height=220 border-width=0 override-redirect=true(0x01)

The 'x' never drops below '100' (which is my test panning). To me this sort of hints the problem occurs not in any of the (many) conversions 'late' in view/* and widget/*, but earlier - perhaps around where the location of the mouse pointer is determined and set as the desired popup location? I haven't quite figured out how that part works though, somewhere in layout/generic/* or layout/xul/* I guess?

@florolf
Copy link
Contributor

florolf commented Dec 13, 2020

It seems that this problem is somehow related to notion not setting _NET_FRAME_EXTENTS.

gtk has this code that tries to determine the extents through _NET_FRAME_EXTENTS first, but falls back to walking the window hierarchy if those are not present. In the case of notion, it ends up at the window corresponding to the WFrame containing the window in question, which seems correct. However, this frame is a child to a virtual root, so the coordinates we get in line 3353 are still not absolute, which somehow contributes to that problem.

Note that this commit added a separate XTranslateCoordinates step to the _NET_FRAME_EXTENTS path as a fix for a similar problem, but did not add the same code to the fallback path that gets triggered in the notion case. Manually adding such a call to that path almost fixes the problem for me. However, there is still a small fixed offset to the menu.

I haven't yet understood where this all ties into how Firefox creates its context menus. That GTK function does not get called at the moment a context menu is created, so it seems that it just contributes to some internal state either in Firefox or GTK.

However, there seems to be an easier (and, ostensibly, more correct) way out of this problem: Setting _NET_FRAME_EXTENTS through notion resolves the problem completely for me (and also fixes some glitches in Java applications). It's not clear to me what the actual intention of that property is (from the point of view of an application/GUI toolkit) or what we should set it to. This discussion suggests that it should indeed be the size of the decorations that get added to the frame. Interestingly, the exact values do not seem to matter w.r.t. resolving this issue.

I'm currently using this hack for testing:

diff --git a/ioncore/netwm.c b/ioncore/netwm.c
index 8fb6ebeb..488e3844 100644
--- a/ioncore/netwm.c
+++ b/ioncore/netwm.c
@@ -38,8 +38,9 @@ static Atom atom_net_wm_allowed_actions=0;
 static Atom atom_net_wm_moveresize=0;
 static Atom atom_net_wm_window_type=0;
 static Atom atom_net_wm_window_type_dialog=0;
+static Atom atom_net_frame_extents=0;
 
-#define N_NETWM 9
+#define N_NETWM 10
 
 static Atom atom_net_supported=0;
 
@@ -68,6 +69,7 @@ void netwm_init()
     atom_net_wm_moveresize=XInternAtom(ioncore_g.dpy, "_NET_WM_MOVERESIZE", False);
     atom_net_wm_window_type=XInternAtom(ioncore_g.dpy, "_NET_WM_WINDOW_TYPE", False);
     atom_net_wm_window_type_dialog=XInternAtom(ioncore_g.dpy, "_NET_WM_WINDOW_TYPE_DIALOG", False);
+    atom_net_frame_extents=XInternAtom(ioncore_g.dpy, "_NET_FRAME_EXTENTS", False);
 }
 
 
@@ -85,6 +87,7 @@ void netwm_init_rootwin(WRootWin *rw)
     atoms[6]=atom_net_active_window;
     atoms[7]=atom_net_wm_allowed_actions;
     atoms[8]=atom_net_wm_moveresize;
+    atoms[9]=atom_net_frame_extents;
 
     XChangeProperty(ioncore_g.dpy, WROOTWIN_ROOT(rw),
                     atom_net_supporting_wm_check, XA_WINDOW,
@@ -174,6 +177,10 @@ void netwm_update_allowed_actions(WClientWin *cwin)
 
     XChangeProperty(ioncore_g.dpy, cwin->win, atom_net_wm_allowed_actions,
                     XA_ATOM, 32, PropModeReplace, (uchar*)data, n);
+
+    long extents[4] = {0};
+    XChangeProperty(ioncore_g.dpy, cwin->win, atom_net_frame_extents,
+                    XA_CARDINAL, 32, PropModeReplace, (uchar*)extents, 4);
 }

@raboof
Copy link
Owner

raboof commented Dec 15, 2020

Wow, that's awesome detective work and indeed seems to have the intended effect here, as well! Let's see if we can easily fill in the actual sizes of the decorations - and then I think this might warrant a 4.0.2 release ;)

@florolf
Copy link
Contributor

florolf commented Dec 15, 2020

I've looked into this some more yesterday. The only way gdk_x11_window_get_frame_extents is called is from gdk_window_get_root_origin (which is only called from Firefox in widget/gtk/nsWindow.cpp) which is supposed to return the upper left corner of the frame (i.e. including decorations). The cause for the confusion above was that Firefox itself also queries _NET_FRAME_EXTENTS and uses that to undo the decoration offset (which is why the actual value of _NET_FRAME_EXTENTS doesn't matter in this case). This causes the offset observed in the fallback path with added XTranslateCoordinates above: That code now correctly works out the frame coordinates, but Firefox is unable to compensate for the decorations because _NET_FRAME_EXTENTS is not available/correct.

In short:

  • It seems that GNOME/gtk@d06e3a6 is indeed incomplete.
  • notion should just set _NET_FRAME_EXTENTS (and probably respond to _NET_REQUEST_FRAME_EXTENTS) correctly, i.e. with the size of the frame's decoration.

I was going to look into creating a patch to implement this properly today, but I find the ion codebase a little hard to work with, so any pointers would be appreciated.

@raboof
Copy link
Owner

raboof commented Dec 17, 2020

I was going to look into creating a patch to implement this properly today, but I find the ion codebase a little hard to work with, so any pointers would be appreciated.

I think something along the lines of your patch above might be fine for now. Actually including the decorations size would of course be better, but this is not so easy since they're part of the frame (the parent of the client window) and can vary depending on the frame state. For now, perhaps setting them to 0,0,0,0 is better than nothing. I'd just suggest to move setting the property to its own method instead of adding it to netwm_update_allowed_actions .

raboof added a commit that referenced this issue Dec 19, 2020
Should fix #59

Co-Authored-By: Florian Larysch <[email protected]>
raboof added a commit that referenced this issue Dec 21, 2020
@wilhelmy
Copy link
Collaborator Author

Thanks a bunch. The end of an era.

I'll update my notion and check whether this fixes any of the other issues I've been having with subwindow positions (I remember openscad).

@pigmej
Copy link
Contributor

pigmej commented Dec 21, 2020

I have updated and ... it feels weird that finally, my firefox windows are appearing on the proper screen...

But definitely works. I have not observed issues so far.

@molecular
Copy link

molecular commented Dec 21, 2020

oh wow. Thanks a LOT to everyone!!

What version will this be released in?

@raboof
Copy link
Owner

raboof commented Dec 22, 2020

What version will this be released in?

I released 4.0.2 with this fix

@wilhelmy
Copy link
Collaborator Author

wilhelmy commented Mar 4, 2021

@florolf Out of curiosity (since this has been bothering us for ages), how did you manage to figure this out?

@florolf
Copy link
Contributor

florolf commented Mar 14, 2021

how did you manage to figure this out?

You expect me to remember after more than a month? ;)

I'm not sure about the exact sequence of events, but I started out with staring at x11trace captures of firefox opening that popup window, but that wasn't really conclusive. I also started reading through the relevant parts of the Firefox codebase, but this is difficult when you don't really know what you're looking for, especially since this could be caused by some state that's only indirectly affecting the popup.

#59 (comment) gave the crucial hint that the _NET_VIRTUAL_ROOTS property influences this behavior somehow, so I looked for usage of this in the Firefox code (doesn't come up there) and in GTK, which led to the gdk_x11_window_get_frame_extents function. Following what it does when running in these particular circumstances leads one to realize the problems discussed in #59 (comment). The GTK commit referenced in that issue basically confirmed this hypothesis.

However, this was not the whole picture, as just fixing the address translation in GTK results in incorrect behavior (the small offset mentioned in that comment). This still bugged me so I followed this back to the Firefox code, which breaks the abstraction and explicitly depends on _NET_FRAME_EXTENTS (see #59 (comment)) to undo the translation done by GTK, negating the usefulness of that fallback path. Thus, we ended up implementing _NET_FRAME_EXTENTS instead.

It also seems that GTK4 does away with that code (or rather the public interface to it: GNOME/gtk@cb23d40 GNOME/gtk@987e787), but this represents a breaking API change, so the Firefox people will have to adapt their code as well. So it remains to be seen how this will break in the future.

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

Successfully merging a pull request may close this issue.