Closed
Bug 601545
Opened 14 years ago
Closed 13 years ago
Clear Recent History dialog jumps downwards and to the right each time details panel is opened/closed
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b9
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: dholbert, Assigned: karlt)
References
Details
(Keywords: regression)
Attachments
(4 files)
4.04 KB,
patch
|
Details | Diff | Splinter Review | |
2.69 KB,
patch
|
roc
:
review+
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
6.07 KB,
patch
|
roc
:
review+
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
1020 bytes,
patch
|
roc
:
review+
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE: 1. Ctrl+Shift+Del (or Tools | Clear Recent History) 2. Click the arrow next to "Details" to toggle details open/closed. 3. Repeat step 2. ACTUAL RESULTS: Each time I toggle "Details", the dialog moves downwards by about the height of the titlebar. REGRESSION RANGE: Works: 20100727030231 0ed73d56efd5 Broken: 20100728030233 beab39d49de9http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0ed73d56efd5&tochange=beab39d49de9 From that range, bug 552982 strikes me as most likely to be able to have caused this. Tentatively marking as blocking that bug, and filing this in that bug's component.
Reporter | ||
Comment 1•14 years ago
|
||
> REGRESSION RANGE: > Works: 20100727030231 0ed73d56efd5 > Broken: 20100728030233 > beab39d49de9http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0ed73d56efd5&tochange=beab39d49de9 [Darn it... In the textbox where I typed that comment, there was definitely no newline before "beab...", and there *was* a newline before the "http://hg.mozilla...", but they got stripped in the posting somehow.] That should have appeared like so: Works: 20100727030231 0ed73d56efd5 Broken: 20100728030233 beab39d49de9 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0ed73d56efd5&tochange=beab39d49de9
Reporter | ||
Comment 2•14 years ago
|
||
Might also have been caused by bug 412773...
Reporter | ||
Comment 3•14 years ago
|
||
Nope, I was right in comment 0 -- this is from bug 552982. I confirmed through targeted local builds that the first affected changeset is: http://hg.mozilla.org/mozilla-central/rev/60fa04517a6f
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 4•14 years ago
|
||
Is this with compiz? I don't seem to be able to reproduce with kwin. Can you check metacity, please?
Reporter | ||
Comment 5•14 years ago
|
||
I can reproduce both with & without compiz. (Restarted firefox in between switching window managers, just in case)
Reporter | ||
Comment 6•14 years ago
|
||
That is to say -- I can reproduce this with Compiz & also with Metacity.
Reporter | ||
Comment 7•14 years ago
|
||
Did a little GDB experimentation (with desktop effects disabled - i.e. using Metacity). After toggling the arrow, I get a series calls to nsWindow::GetScreenBounds(), the fourth of which is the interesting one. All of them (up to that point) take the gdk_window_get_root_origin() path, and here's what gdk_window_get_root_origin() does in each case... - The first returns with: x=202 y=486 - The second has a different |this|, and returns: x=0 y=0 - The third returns with: x=202 y=486 - The fourth returns with: x=203 y=513 Also: the fourth "gdk_window_get_root_origin" call is what synchronously moves the dialog. (That is: I'm stepping through GDB, with no paints happening in Firefox. When I step over that gdk_window_get_root_origin() line, the dialog snaps to its new position.)
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > Also: the fourth "gdk_window_get_root_origin" call is what synchronously moves > the dialog. (That is: I'm stepping through GDB, with no paints happening in > Firefox. When I step over that gdk_window_get_root_origin() line, the dialog > snaps to its new position.) I'm guessing what's happening here is that a previous async move request is being flushed when gdk_window_get_root_origin() makes a synchronous request. i.e. it could well be an earlier operation that moved the window.
Assignee | ||
Comment 9•14 years ago
|
||
Running with --sync makes all requests synchronous.
Assignee | ||
Comment 10•14 years ago
|
||
I can reproduce with metacity. Once it reaches the bottom of the screen it no longer jumps down even when shrinking, but continues to jump right until it reaches the right-hand edge. If moved to overlap either border of the screen it continues to move in both directions.
Summary: Clear Recent History dialog jumps downwards each time details panel is opened/closed → Clear Recent History dialog jumps downwards and to the right each time details panel is opened/closed
Assignee | ||
Comment 11•14 years ago
|
||
There seems to be general confusion in the GTK port about whether mBounds is client coordinates or window-manager frame coordinates. Part of this confusion may be due to the different interpretation of position and size in move/resize requests: positions are window-manager frame origins, but sizes are client dimensions. http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.5 In http://hg.mozilla.org/mozilla-central/rev/60fa04517a6f the origin of mBounds was changed to that of the client window (which for non-popup-with-titlebar windows was a result of my review request) for consistency with other platforms (bug 552982 comment 41), but that is not what is wanted for gtk_window_move.
Assignee: nobody → karlt
Component: XP Toolkit/Widgets: XUL → Widget: Gtk
QA Contact: xptoolkit.xul → gtk
blocking2.0: ? → final+
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11) > In http://hg.mozilla.org/mozilla-central/rev/60fa04517a6f > > the origin of mBounds was changed to that of the client window (which for > non-popup-with-titlebar windows was a result of my review request) for > consistency with other platforms (bug 552982 comment 41) ... I don't think that was making the behaviour consistent with other platforms. nsView::DoResetWidgetBounds uses nsIWidget::GetBounds to obtain the current position for comparing directly with new bounds passed to nsIWidget::Move. AFAICT nsGlobalWindow and nsXULWindow use the same coordinates for nsIWidget::Move as come from GetScreenBounds, so Move must take the frame top-left, and so GetBounds should return the frame top-left.
Assignee | ||
Comment 13•14 years ago
|
||
It took me a little while to understand why (noautohide) panels with titlebars did not respond well to making GetBounds return the window-frame position. When an NS_MOVE event is dispatched, nsXULPopupManager::PopupMoved responds by moving the box frame and view. The key thing preventing an infinite loop is that moving the view does not move the widget when widget->GetBounds returns the same position as the new coordinates. That doesn't work out for context menus because nsMenuPopupFrame::SetPopupPosition adds offsetForContextMenu to the coordinates from the NS_MOVE event when calculating the new position for the view. This would cause the context menu to always iterate ad infinitum towards the bottom right of the screen, except that NS_MOVE events are not dispatched for context menus. This is due to three conditions being satisfied: 1) When OnConfigureEvent does not dispatch NS_MOVE events if the position in the event matches the position recorded in mBounds from the last Move or Create. 2) We must be either only positioning context menus once or always receiving the configure event for the first position before changing the position. If the ConfigureNotify event for the first position of the window were received after the window was moved, the context menu would be moved back to the first position + offsetForContextMenu, with potential subsequent iterations. 3) The context menu is an override-redirect window so the window manager will not move the window. All moves are at Gecko's direction. Things are even more snazzy for noautohide titlebar panels. When a ConfigureNotify event is received for these (after the window manager has positioned the window), 1. mBounds is changed to point to the client window (not frame) top-left. 2. The NS_MOVE event is dispatched with client top-left coords. 3. The view top-left is aligned with these client window coords. 4. The view tries to move the widget frame top-left to the current client window coords. This would move the window right and down but for window->GetBounds now lying to say that it's frame top-left has already moved. 5. The box frame is positioned in the client window coords indicated by the NS_MOVE event. This is why the NS_MOVE event is necessary for these popups. I think we can implement something for GetClientOffset() (used for step 5, but currently returns zero), and so hopefully the NS_MOVE event can carry the window-frame top-left and the widget does not need to lie about its position to give the box frame its appropriate offset.
Assignee | ||
Comment 14•14 years ago
|
||
blocking2.0: final+ → -
Assignee | ||
Comment 15•13 years ago
|
||
The patches I'm attaching are as far as I had got before this bug was removed from the blocking2.0 list. They do not address the core issues described in comment 12 and 13 but I think this is the right thing to do and it remedies the reported symptoms.
Attachment #498912 -
Flags: review?(roc)
Assignee | ||
Comment 16•13 years ago
|
||
Currently things are confusing with 3 variables mNeedsResize, mNeedsMove, and mPlaced having overlapping meanings. This patch reduces this to only mNeedsResize and mNeedsMove.
Assignee | ||
Updated•13 years ago
|
Attachment #498914 -
Flags: review?(roc)
Assignee | ||
Comment 17•13 years ago
|
||
This is a documentation-only change to clarify some of the confusion leading to the issue described in comment 12.
Attachment #498919 -
Flags: review?(roc)
Attachment #498912 -
Flags: review?(roc) → review+
Attachment #498914 -
Flags: review?(roc) → review+
Attachment #498919 -
Flags: review?(roc) → review+
Attachment #498912 -
Flags: approval2.0+
Attachment #498914 -
Flags: approval2.0+
Attachment #498919 -
Flags: approval2.0+
Assignee | ||
Comment 18•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ec338170e5f7 http://hg.mozilla.org/mozilla-central/rev/71d2d1c6d12d http://hg.mozilla.org/mozilla-central/rev/944010571e9d http://hg.mozilla.org/mozilla-central/rev/0489b54d5ce8
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b9
You need to log in
before you can comment on or make changes to this bug.
Description
•