30.11 KB, text/plain
30.52 KB, text/plain
1.52 KB, patch
|Details | Diff | Splinter Review|
Sometime between "hg update -d 2011-07-11" and changeset 2ff6999222c2 (on 2011-7-12) my m-c build started failing to be able to open any window menus or open context menus. Tried both clobber and removing the entire obj directory to rule out a build issue. OS: Ubuntu 8.04 Running via remote X Options: . $topsrcdir/browser/config/mozconfig ac_add_options --disable-debug mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@-opt # mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@-debug mk_add_options AUTOCONF=autoconf2.13 mk_add_options MOZ_MAKE_FLAGS="-s -j4" ac_add_options --enable-jprof ac_add_options --disable-strip ac_add_options --with-ccache=/usr/local/bin/ccache ac_add_options --enable-debugger-info-modules ac_add_options --enable-optimize="-g -O3 -fno-omit-frame-pointer" Bisecting.
Done with bisect, changeset http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=d6e1995bcdd6 The first bad revision is: changeset: 72599:d6e1995bcdd6 user: Karl Tomlinson <firstname.lastname@example.org> date: Fri Jul 08 15:14:27 2011 +1200 files: widget/src/gtk2/nsWindow.cpp description: b=624329 skip NS_MOVE dispatch on ConfigureNotify on override-redirect windows to work around nsXULPopupManager::PopupMoved moving the context menus again r=roc Makes some sense since it touches GTK window code Note: remote X is to a WinXP machine running X under cygwin. Wouldn't be shocked if some other remote-X setups are affected as well. regression -> karlt
Confirmed that backout of that changeset makes tip function correctly.
Let me know if you want me to try a patch or get more info.
Seems like something we should track for 8 to make sure we don't ship this!
If we want this for 8, we need to get traction on it!!! Joe? Karl? I have to run my builds with a backout patch...
Karl's been on vacation until today. so hasn't had an opportunity to look into it :)
Can you confirm I understand the setup correctly, please? Firefox is running on the Ubuntu system. The display is an X server running under cygwin. What window manager is running on the X server? I'm not familiar with X servers running under cygwin. I assume it must be some kind of special server. Can you point me at info on that? Can you attach a log of a minimal session reproducing the problem in a debug build (or force the logging in an opt build) with NSPR_LOG_MODULES=Widget:5,WidgetFocus:5 in the environment, please?
(In reply to comment #7) > I'm not familiar with X servers running under cygwin. I assume it must be > some kind of special server. Can you point me at info on that? http://x.cygwin.com/ Are you using Multi-Window mode? Do you get different behavior in other modes?
Correct: Firefox is running on Ubuntu Display is X server on cygwin I'm running multi-window mode (X windows rehosted as Win32 windows) I have not tried other modes. About Cygwin/X says Version 1.8.2; build date 2010-08-06 I'm rebuilding my debug build without the backout. I'll attach a log *with* the backout now for comparison.
Created attachment 549364 [details] Log with backout patch removed (newer build) This shows the problem. Startup, click File, right-click in content, exit. Note: the first log was an existing debug build I had (with the backout I believe); it was probably a week or so old. This is a fairly fresh pull, with the backout popped off. I'll pop the backout back on and do a new log to replace the earlier one.
Created attachment 549379 [details] Log of debug build without backout Log without the backout, otherwise same source/build.
Thanks, Randell. Looks like there are some spurious ConfigureNotify events that we wouldn't get from a "real" X server. You were probably being saved by this early return skipping check_for_rollup: http://hg.mozilla.org/mozilla-central/rev/d6e1995bcdd6#l1.81 That was working, even though it was comparing apples and oranges, for second and subsequent duplicate configure events because mBounds is getting set incorrectly later in the function.
Created attachment 550889 [details] [diff] [review] revert removal of early return, to avoid unwanted rollup on spurious ConfigureNotify Can you confirm that this resolves the problem for you, please, Randell?
Verified that this patch fixes the problem; thanks!
not going to track this for our releases since this is not a common supported use case.