797547 - After clearing Downloads Panel list, panel pops up way to the left of the Downloads toggle button.

Status: RESOLVED WORKSFORME

(Core :: Widget: Gtk, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

On Ubuntu Precise using Nightly (Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0

STR:

1) Start a new download
2) Cancel the download once the Download Panel appears by clicking on the "X" next to the download item
3) Right click on the panel, and choose to clear the list.
4) Click on the Downloads toggle button to hide the panel.
5) Click on the Downloads toggle button again to show the panel.

What happens?

The Downloads Panel appears way too far to the left of the Downloads toggle button. For me, the panel appears on the far left side of the search input. Screenshot coming next.

What's expected?

The panel should still be anchored to the Downloads toggle button.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: This is where the panel ended up

Comment 2

[Mike Conley (:mconley) (:⚙️)]

I've been tinkering with this today, and it looks like the new position's left-most edge matches the left-most edge of the Downloads Panel when we have items listed in it.

When I tinker with attributes on the panel that might cause a layout refresh (for example, setting the hasdownloads attribute to "true", displaying the popup, and then removing the attribute), the panel starts showing up in the right place.

And to reiterate, I'm only seeing this on Ubuntu (Precise) - not on Windows or OSX.

Neil - any idea what might be going on?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Ah hah - so I think this is somewhat related to how the empty panel (which just shows "Show All Downloads") on Linux is quite a bit thinner than its counterparts on Windows and OSX.

If, for example, I set noautohide to true to the downloads panel on Windows, and then clear the list of stopped downloads, I can reproduce this problem on Windows.

Doesn't work for OSX, so I'm wondering if there's a subtle layout-refresh bug we're seeing here.

Regardless, a simple fix would be to widen the empty downloads popup for Linux. I'm waiting to hear back from shorlander about how these panels are supposed to appear when empty.

Comment 4

[david.smitmanis]

Could this be related to Bug 751947?

Does it still happen now when that issue is fixed?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to david.smitmanis from comment #4)
> Could this be related to Bug 751947?
> 
> Does it still happen now when that issue is fixed?

Yes, this still happens with bug 751947 landed.

I've also confirmed that Linux and OSX have the empty download panel "right" - it needs to have a smaller width.

See http://cl.ly/442a3U0d1M1B1b411o12/o

This means that we can't solve this by simply persisting the "has downloads" width. It also means that we have to fix this for Windows, which currently persists the width.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I've pretty sure the SocialAPI team him this too - see bug 798226 and bug 798212.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mike Conley (:mconley) from comment #6)
> I've pretty sure the SocialAPI team him this too - see bug 798226 and bug
> 798212.

typo - "him" should be "hit".

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Neil - any luck pinning this down on your side?

Comment 10

[Marco Bonardo [:mak]]

Do you still needinfo from Neil here?

Comment 11

[Neil Deakin]

This is a different bug.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Whoops, OK. Moving those resources I attached to bug 798226 back to this bug.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Attachment: Widget log from YouTube video

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Attachment: Last 250 lines from a repeat run of the STR

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Karl:

Sorry to be such a pest about this bug!

Anyhow, apparently this bug is distinct from bug 798226, which I guess makes it less complicated.

Are any of the resources I've attached useful? Is there anything else I can do to help determine what's going wrong here?

-Mike

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Some important points from Neil from bug 798226:

"When I debug this I see the popup being resized and positioned by nsMenuPopupFrame correctly. When it opens and displays incorrectly, I see:

- calls to nsWindow::Resize and nsWindow::Move which move the shrunk popup into the new place with the arguments having the correct expected values. In my test, this position is (724, 135)
- the popup is opened and the the popup positioned. The coordinates look correct, although it is now (723, 135). Perhaps some margin got added?
- call to nsWindow::Show to show the window. mBounds is set to (724, 135, 155, 53) which is correct.
- OnConfigureEvent is called with *incorrect* event values: (357, 135, 155, 53). The '357' value is what the old x-position was before the popup was resized.

So, there is an extra OnConfigureEvent event being sent with incorrect values. It's interesting that the size is being changed while the popup is closed though. Maybe gtk doesn't like that happening?"

Comment 17

[Mike Conley (:mconley) (:⚙️)]

And from Karl:

"Is mListenForResizes set on the nsWindow?
If so, the Resize will be effected immediately (instead of delayed until just before NativeShow).  That would make the Resize happen before the Move.
Did that Resize change the position or only the size?

If that Resize only changed the size and mListenForResizes is set then I'd expect a configure-event signal first in response to the Resize and then another in response to the Move.  Is there a subsequent OnConfigureEvent with the correct values?"

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Hey Chris - do you have any idea what might be happening here?

Comment 19

[Karl Tomlinson (:karlt)]

The log suggests there's no function call made to move the popup between the hide and show, so I don't know what is moving the window.

The popup is not noautohide and so has override-redirect so the window manager won't be moving it, and I can reproduce with both kwin and metacity.

I wonder whether GTK might be storing the position used at the previous show.

Comment 20

[Karl Tomlinson (:karlt)]

This is looking like a GTK bug, with the workaround being to reposition the window again before the show if it has moved while shown.  Widget code seems the best place for such a workaround.

Comment 21

[Mike Conley (:mconley) (:⚙️)]

(In reply to Karl Tomlinson (:karlt) from comment #20)
> This is looking like a GTK bug, with the workaround being to reposition the
> window again before the show if it has moved while shown.  Widget code seems
> the best place for such a workaround.

Cool - thanks Karl.

Having never really worked down in our widget code, can I hand this off to your or someone else? Or, alternatively, can you guide me on where and how I need to implement this workaround?

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Karl:

Hey - any tips for me?

-Mike

Comment 23

[Karl Tomlinson (:karlt)]

I'm guessing its possible to reuse mNeedsMove, but I haven't looked into it.

Mike, if you are able to write a test (or adjust the test in bug 798226 to also test this), I expect I could put together a fix.

Comment 24

[Mike Conley (:mconley) (:⚙️)]

So this is really strange - today, I noticed that I'm not longer able to reproduce this issue. I thought that, perhaps, the bug had been fixed in platform somewhere...but running some Nightlies from last week (where the bug definitely existed) did not reveal the bug again.

I did run a series of updates for Ubuntu, which may have had something to do with it.

Regardless, this bug is mysteriously no longer an issue for me. Marking as WORKSFORME until someone can reproduce it.

Comment 25

[Karl Tomlinson (:karlt)]

It may have been https://hg.mozilla.org/mozilla-central/rev/a9500b386854 that made the STR work as expected.  When the MozContainer GtkWidget was hidden, it called gtk_widget_queue_resize, which ended up setting need_resize on the parent GtkWindow.

If needs_resize is set, gtk_window_show calls gdk_window_move_resize (which positions as well as resizes).
IIRC gtk_window_move_resize only moves if the configure_request.x or .y has changed.

When the window is not shown, gtk_window_move updates initial_x and _y, which gets used for the next configure request.  However, when the window is shown, gtk_window_move does not do anything similar to ensure that configure_request.x and .y are updated.  That seems to be a GTK bug.

This may still affect us if the window is moved while shown, hidden, resized but not repositioned, and shown again.

Comment 26

[Mike Conley (:mconley) (:⚙️)]

Well, it seems to fix the most common case (clearing the downloads panel), so I'm pretty happy. I haven't found any other cases where the problem manifests again.

Accidental bugfix ftw!