From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc2) Gecko/20020520 Debian/1.0rc2-3 BuildID: 20020520 Debian/1.0rc2-3 Popup menus in the content area have an offset in y direction if the page was loaded without visible tab bar and after loading a new tab was opened. When the page on the old tab is reloaded (or a new url is opened) the menu is back at the correct position. This bug also does not happen, when the menu has not been opened before the new tab was added. This also applies to menus for <input> elements and can cause accidental selections. Reproducible: Always Steps to Reproduce: 1.Open a new navigator window 2.Right-click into the content area 3.Open a new tab (so that the tab bar shows up) 4.Go back to the old tab 5.Right-click again. Actual Results: Menu pops up at a wrong position, offset by about the height of the tab bar.

This happens with combo boxes too.

same with the sidebar (open menu, toggle sidebar, open menu again)

Reason is that (widget/src/gtk/)nsWindow::GetWindowPos() uses cached x/y coords and obviously these cached values are not invalidated when tabs or sidebars show up/vanish.

This patch removes the position cache (that seems to be implemented only with gtk, e.g. gtk2 doesn't cache) and therefore fixes the problem. The reason why page reloading cured the symptom was not that the cache was invalidated but rather a completely new nsWindow object was created.

=> blizzard, fast blast for 1.0 or 1.0.1. /be

I'd like to ask for a review of attachment 85173 [details] [diff] [review] Thanks

Wait, the position cache is there for a reason. Why not find the problem in the cache instead of removing it wholesale?

As far as I can see, the cache invalidation is not working for the following reasons: - the gtk event handler that invokes InvalidateWindowPos() is only triggered, when the whole mozilla window is moved or when it is resized (even without changing the (x,y) position). - nsWindow::Move() invalidates the cache only when the position changed but viewing/hiding the sidebar/tabbar/main menus triggers a call to Move() with unchanged coordinates (always 0,0). Obviously that document window is contained in another window. This containing window is indeed moved to the correct coordinates and calls gdk_window_move(mSuperWin->shell_window, aX, aY) in its nsWindow::Move() method. This gdk_window_move() does not trigger the InvalidateWindowPos event. So I think the reason for this bug is that gdk_window_move() must invalidate all position caches of all contained windows. As I am quite unfamiliar with gdk/gtk I have no idea about what needs to be done to have this event triggered.

Because signal toplevel_configure is only attached to toplevel widget, position or size change of non-toplevel widget won't trigger the change of cached position coordination, e.g, sidebar open/close. We can't attach every widget to the configurenotify of their every ancestor because it is too inefficent. To traverse all the child while get ConfigNotify to invalidate the cache is also inefficent. So I choose to invalidate the cache in Move method. Althoug some Move methed does not actually cause a move, this way at least saves the cache and avoid a castcade invalidating. Fortunately, layout calls the Move of the child widget when it's ancestor does a move. Seeking review=?

Comment on attachment 85173 [details] [diff] [review] patch, getting rid of the position cache Would be ok with me, much better than having no cache at all.

*** Bug 94296 has been marked as a duplicate of this bug. ***

I duped bug 94296 (with a bunch of dups) to this one. You don't have to open the menu/widget first before causing the toolbar/etc to collapse/appear to see the problem. I.e. With sidebar on Go to page with dropdown select widget Turn off sidebar (F9) Open dropdown Let's get reviews on this and determine if it's the right solution

I'd like to give a FAQ of this bug to push the review: 1. When does widget update its cached location coordinate? Only when the widget revieves the toplevel_configure signal or someone calls InvalidateWindowPos explicitly. ( See handle_invalidate_pos in widget/src/gtk/nsWindow.cpp) 2. Who will send the toplevel_configure signal? When the toplevel widget revieces the ConfigureNotify, it will send out the signal. ( See attach_toplevel_listener and toplevel_window_filter in widget/src/gtksuperwin/gtkmozarea.c) 3. Will the cached position be invalidated when a non-toplevel parent widget get the position change? (e.g. sidebar show/hide, toolbar show/hide will cause the position of the other widgets changed) No. 4. Why mozilla for windows don't have such problem? It does not use such cache. ( See nsWindow::Move in widget/src/window/nsWindow.cpp )

Comment on attachment 98852 [details] [diff] [review] patch r=blizzard

Comment on attachment 98852 [details] [diff] [review] patch sr=bryner

Comment on attachment 98852 [details] [diff] [review] patch a=asa for checkin to 1.2 (on behalf of drivers).

checked in.

*** Bug 53318 has been marked as a duplicate of this bug. ***

*** Bug 112853 has been marked as a duplicate of this bug. ***

*** Bug 118424 has been marked as a duplicate of this bug. ***

Note that this did not fix <select>s inside <iframe>s...