Community
Participate
Working Groups
Created attachment 249533 [details] code reproducing the issue The problem was observed for gtk2-2.24.23-6. Steps to reproduce: 1. Create a shell, a childShell for that shell and a control (e.g. label) inside the shell. Open the shells. 2. Move the childShell to a new location (locationXXX) 3. Set the visibility of the childShell to false 4. Reparent the control from shell to childShell 5. Set the visibility of the childShell to true As a result, when the childShell is shown again, it will be reset to its default position. The expected behaviour would be to show it in the last location (locationXXX), where it was before it was hidden. The same problem occurs if the original parent of the control is the shell and on step 4 the control's parent is set to childShell. I've attached a small program that can be used to reproduce the issue. Steps: - push the "Invisible" button - push "Reparent" button - push "Visible" button The error seems to come from gtk (the position of the shell is reset if a control is reparented). I can also share a plain gtk code sample with you, exemplifying the issue - if you need it.
Interesting observation and thank you for the test case. I can reproduce the issue just by setting the widget to be invisible [setVisable(false)] and then setting it visible again [setVisable(true)]. (I.e, reparenting doesn't seem to be necessary to reproduce the issue). This is reproducible on gtk2 and gtk3.14, Fedora 21. Once the critical gtk3 bugs are squashed: Bug 441566 - [GTK3] Improve support for newer versions of GTK+ 3 (>= 3.10.x) We will work on fixing this also. (unless someone has time to work on this in the meantime). Thank you
I could not reproduce the issue you described - I tried to switch the visibility of both the label and shells with the same correct result. I only experience the issue when reparenting. We tested this in plain gtk and the issue comes from there: the window's position gets reset after a reparenting takes place.
Hi Leo, we would like to help solve this issue. Currently our only idea is to track the reparenting, by checking whether a reparenting is happening while one of the affected shells is invisible. This also would mean keeping track of the shell location by ourself. Maybe you have some idea how to solve this issue more elegant? Otherwise I would provide a gerrit commit containing our solution as soon as it is ready. Cheers, Eugen
Hello Eugen, By all means, please go ahead with patch. The DND stuff has been keeping me occupied. Gtk is somewhat notorious for not keeping size data. e.g in Control:setParent() there is a note about it: /* * Restore the original widget size since GTK does not keep it. */ resizeHandle(width, height); I had similar issues in DateTime, and in the end I ended up caching size data (see DateTime:computeSize()) I'd be happy to review/test the patch.
New Gerrit change created: https://git.eclipse.org/r/42910
Hi Leo, I implemented a solution that fixes the bahavior for me. Please let me know if the patch needs to be refactored etc. Cheers, Eugen
I'll try to test/review today or early next week.
New Gerrit change created: https://git.eclipse.org/r/42930
(In reply to Eugen Neufeld from comment #6) > Hi Leo, > I implemented a solution that fixes the bahavior for me. > Please let me know if the patch needs to be refactored etc. > > Cheers, > Eugen Thank you for the investigation and writing a patch to fix. It is most useful. I tried out your patch and it did work. My only concern was that we already have the 'oldX/oldY/moved' workaround (1*) and the newly added workaround has variables: 'posX/posY/updatePosition' that sort of duplicate the functionality of the existing workaround. So as a result we have~: Workaround * Workaround = Workaround^2 :-). I investigated a little. There seems to be two issues with the existing workaround: 1) We handle a configure-event (*2) even if the shell is invisible, and this messes with the oldX/oldY position. 2) We don't re-position the shell when the shell is made visible again from being invisible. In the interest of maintainability; based on your patch, I merged the functionality of your patch with the old patch. I couldn't amend to the patch as I don't have contributor rights, but I made a separate commit: https://git.eclipse.org/r/#/c/42930/ @Alexandra/Eugene, Would you be able to inspect the patch and let me know your thoughts? Thank you 1* This workaround moves the shell back into place after a minimize/maximize event. 2* A configure event is a GTK event that is sent when a window is moved/resized: https://developer.gnome.org/gtk3/stable/GtkWidget.html#GtkWidget-configure-event
Hi Leo, thank you for refactoring the patch. I already thought that the new fix is defining variables that are already there. But I still don't understand how the oldX/oldY values are working, as those variables are set but never read. I will try out the patch on Monday morning. Thanks, Eugen
Hi Leo, your patch works perfectly for me. Do you have an idea whether this might be part of the Mars release? Cheers, Eugen
(In reply to Eugen Neufeld from comment #10) > Hi Leo, > thank you for refactoring the patch. > I already thought that the new fix is defining variables that are already > there. > But I still don't understand how the oldX/oldY values are working, as those > variables are set but never read. > I will try out the patch on Monday morning. > > Thanks, > Eugen Well, they are used in gtk_configure_event, to determine if an SWT.Move event should be triggered: if (!moved || oldX != x [0] || oldY != y [0]) { //<< here moved = true; oldX = x [0]; oldY = y [0]; sendEvent (SWT.Move); // widget could be disposed at this point } This then eventually reaches 'setBounds(..)' and moves the shell around. The submitted patch still allows the user to move the shell when it is invisible, it just blocks 'gtk' from automatically moving it via a config_event when it's invisible. So it shouldn't break anything. Not sure if it can make it's way into Mars, I'll check with Alex.K
Gerrit change https://git.eclipse.org/r/42930 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=827e21430e04d8fe42a25c2caeaa1eeed2cae429
Definetely Mars content. Pushed to master now.
Verified in I20150316-2000