For newcomers, this is a nice bug to fix once you have 1 or 2 fixed.

I believe this happens because we show the popover when it is not attached to the button. So needs to investigate when the popover is shown, and make sure the toolbar and the button are constructed already.

This also happens if I already have two nautilus windows open before the copy/delete operation. In this case, the file operation indicator in nautilus won't disappear on its own even till one nautilus window is closed which is somewhat messy.

I'll have a go at fixing this one. Will hold off writing any code until my other change has been reviewed

(In reply to Neil Herald from comment #3)
> I'll have a go at fixing this one. Will hold off writing any code until my
> other change has been reviewed

I cannot see any change or patch in this bug report.
What do you mean exactly?

I haven't made any changes yet. I'll fix this next, but I won't make any code changes until my change for 747907 has been reviewed. Just want to make sure I'm on the right lines before I write more code.

(In reply to Neil Herald from comment #5)
> I haven't made any changes yet. I'll fix this next, but I won't make any
> code changes until my change for 747907 has been reviewed. Just want to make
> sure I'm on the right lines before I write more code.

Ah! There is not need to hold it back, just go ahead please :)
Each patch has nothing to do with each other and can be reviewed independently.
Also seems Bastien reviewed your patch, so you can have some feedback.

You're right - the window pointer of operations_popover is NULL, when it's created for the second window. 

When the second window is opened, it opens the popover. Is this the desired behaviour though? If the user has started an operation (e.g. a delete) in one window, and the popover has already appeared, I don't think they would want to see the popover appear again for the second window. They would already know that the delete is happening because they got the popover in the first window. 

If the popover shouldn't be shown automatically in the second window, that's an easy change (I have a fix ready for that). Otherwise I can look into fixing it so it opens automatically for the second window (and any other subsequent windows).

(In reply to Neil Herald from comment #7)
> You're right - the window pointer of operations_popover is NULL, when it's
> created for the second window. 
> 
> When the second window is opened, it opens the popover. Is this the desired
> behaviour though? If the user has started an operation (e.g. a delete) in
> one window, and the popover has already appeared, I don't think they would
> want to see the popover appear again for the second window. They would
> already know that the delete is happening because they got the popover in
> the first window. 
> 
> If the popover shouldn't be shown automatically in the second window, that's
> an easy change (I have a fix ready for that). Otherwise I can look into
> fixing it so it opens automatically for the second window (and any other
> subsequent windows).

The popover also doesn't disappear on its own if here are two nautilus windows open. I waited 30 minutes after the file operation was completed...
It only behaves correctly when there is one nautilus window only.

Created attachment 325649 [details]
Button persist bug

I spotted that too. There's also another similar bug where the toolbar button doesn't disappear (see screenshot) in the other windows. When there are two windows, the popover appears on both. You need to close both popovers (by bringing each window in focus, in turn), then the button will stay in the toolbar for the window you brought into focus first. 

I'm planning on making the following changes: 
* Change it so that the popover only automatically appears in the active window (the window that triggered it). I don't think it makes sense to have it appear in more than one window - as I explained before
* If the toolbar button in the other windows still doesn't disappear as it should, I'll raise a separate bug for that

If anyone disagrees with these changes, please let me know. The other thing we could do is close the popover if the window loses focus. That way, only one window can ever have a popover open, and would alleviate some of the issues (and probably be a better for the user).

(In reply to Neil Herald from comment #10)

> I'm planning on making the following changes: 
> * Change it so that the popover only automatically appears in the active
> window (the window that triggered it). I don't think it makes sense to have
> it appear in more than one window - as I explained before
This makes sense.

> If anyone disagrees with these changes, please let me know. The other thing
> we could do is close the popover if the window loses focus. That way, only
> one window can ever have a popover open, and would alleviate some of the
> issues (and probably be a better for the user).
You mean move the popover dynamically back and forth between focused windows?
That would work too. Would the popover also automatically move to the unfocused nautilus window if the focused one is closed?

(In reply to Neil Herald from comment #9)
> Created attachment 325649 [details]
> Button persist bug
Yes, I see this one too. This happens if one window is closed but the other is still there and the file operation ended a long time ago.

(In reply to Hussam Al-Tayeb from comment #11)
> You mean move the popover dynamically back and forth between focused windows?
> That would work too. Would the popover also automatically move to the
> unfocused nautilus window if the focused one is closed?
I hadn't actually thought of showing it on the newly focused window, I think it's a nice idea (although probably not the easiest thing to implement). My suggestion was purely to close the popover if the window lost focus. I tried this out and it works OK, but would be interested to see what others think - I'm new to the project and don't know what design decisions went into it.

(In reply to Neil Herald from comment #7)
> You're right - the window pointer of operations_popover is NULL, when it's
> created for the second window. 
> 
> When the second window is opened, it opens the popover. Is this the desired
> behaviour though? If the user has started an operation (e.g. a delete) in
> one window, and the popover has already appeared, I don't think they would
> want to see the popover appear again for the second window. They would
> already know that the delete is happening because they got the popover in
> the first window. 
> 

The popover is not "only" to inform the user that a operation is going on, but to see the progress. So I believe the user would want to see it in the new window.

(In reply to Neil Herald from comment #9)
> Created attachment 325649 [details]
> Button persist bug

That should be fixed in 3.20, is not?

(In reply to Neil Herald from comment #10)
> I spotted that too. There's also another similar bug where the toolbar
> button doesn't disappear (see screenshot) in the other windows. When there
> are two windows, the popover appears on both. You need to close both
> popovers (by bringing each window in focus, in turn), then the button will
> stay in the toolbar for the window you brought into focus first. 
> 

It's not a bug, it's intended.
We can discuss better approaches to it though.

> I'm planning on making the following changes: 
> * Change it so that the popover only automatically appears in the active
> window (the window that triggered it). I don't think it makes sense to have
> it appear in more than one window - as I explained before

Makes sense I think.

> * If the toolbar button in the other windows still doesn't disappear as it
> should, I'll raise a separate bug for that
> 

Yeah, I think that's a separate issue to discuss.

> If anyone disagrees with these changes, please let me know. The other thing
> we could do is close the popover if the window loses focus. That way, only
> one window can ever have a popover open, and would alleviate some of the
> issues (and probably be a better for the user).

I'm not sure. If the user opened the popover, why should we close it to it?
Or do you mean only in the case where we show the popover because a operation started?

(In reply to Carlos Soriano from comment #14)
> (In reply to Neil Herald from comment #9)
> > Created attachment 325649 [details]
> > Button persist bug
> 
> That should be fixed in 3.20, is not?

I'm on the nautilus master branch, unless that doesn't have the 3.20 changes in?

(In reply to Carlos Soriano from comment #14)
> (In reply to Neil Herald from comment #9)
> > Created attachment 325649 [details]
> > Button persist bug
> 
> That should be fixed in 3.20, is not?

It's not fixed. I still see it in 3.20 release.

(In reply to Carlos Soriano from comment #15)
> (In reply to Neil Herald from comment #10)
> > ... button doesn't disappear in the other windows ...
> > 
> 
> It's not a bug, it's intended.
> We can discuss better approaches to it though.
It just seems strange that it disappears in one of the windows (about a few seconds after the last operation has finished - which is what I'd expect), but it never goes away from the second one. And also the fact it brings up an empty popover. Anyway let's leave the discussion on that to another thread!


> > The other thing we could do is close the popover if the window 
> > loses focus
> 
> I'm not sure. If the user opened the popover, why should we close it to it?
> Or do you mean only in the case where we show the popover because a
> operation started?
I was thinking of doing it for all cases. I'll leave that change out, I wasn't so sure on it after I'd tried it out - for the reason you pointed out. Although, the popover closes when you click elsewhere on the same window (I think, although that's maybe only when you click the toolbar).

An empty popover definitely looks like a bug.
How about show the popover in all windows but also populate it with file operation progress started newly from other windows as well.
If a new nautilus window appears, also add a popover with current progress operations.
Then when the last operation is finished, remove the popovers from all windows.
Sounds like a clean approach? For me, this would be the least confusing.

(In reply to Hussam Al-Tayeb from comment #19)
> An empty popover definitely looks like a bug.
> How about show the popover in all windows but also populate it with file
> operation progress started newly from other windows as well.
> If a new nautilus window appears, also add a popover with current progress
> operations.
> Then when the last operation is finished, remove the popovers from all
> windows.
That's pretty much how it should work. And the list is shared between all windows, so if you start a new operation from another window, it'll be added to the list.

(In reply to Hussam Al-Tayeb from comment #17)
> (In reply to Carlos Soriano from comment #14)
> > (In reply to Neil Herald from comment #9)
> > > Created attachment 325649 [details]
> > > Button persist bug
> > 
> > That should be fixed in 3.20, is not?
> 
> It's not fixed. I still see it in 3.20 release.

:(

(In reply to Neil Herald from comment #18)
> (In reply to Carlos Soriano from comment #15)
> > (In reply to Neil Herald from comment #10)
> > > ... button doesn't disappear in the other windows ...
> > > 
> > 
> > It's not a bug, it's intended.
> > We can discuss better approaches to it though.
> It just seems strange that it disappears in one of the windows (about a few
> seconds after the last operation has finished - which is what I'd expect),
> but it never goes away from the second one. And also the fact it brings up
> an empty popover. Anyway let's leave the discussion on that to another
> thread!
> 

Indeed, that shouldn't happen. Intended behavior is to only remove the button when all windows have the popover closed (there is a code not so beautiful in nautilus-toolbar for that).
> 
> > > The other thing we could do is close the popover if the window 
> > > loses focus
> > 
> > I'm not sure. If the user opened the popover, why should we close it to it?
> > Or do you mean only in the case where we show the popover because a
> > operation started?
> I was thinking of doing it for all cases. I'll leave that change out, I
> wasn't so sure on it after I'd tried it out - for the reason you pointed
> out. Although, the popover closes when you click elsewhere on the same
> window (I think, although that's maybe only when you click the toolbar).

Right, losing focus it's pretty close to "clicking anywhere else". If the change makes sense to you and the code doesn't look ugly... why not.
However, my main concern about "automatic" actions is that usually bring more problems than solutions. In this case I can imagine this: you open the popover to see the finished operation, then you change the window to check something, and then the popover dissapear, and not only that, the button dissapear too losing the ability to check it again. Then the user is angry :)

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

Created attachment 325885 [details] [review]
toolbar: fix broken operations popover when new windows are opened

Opening another Nautilus window while a file operation is in progress causes
the the operations popover to open in a broken state. The associated window
isn't realised at the time the popover is first shown, meaning that the popover
gets it's own window/title bar, which looks odd and doesn't work like it
should.

This fix is to delay the initialisation of the operations popover and button
until the toolbar (and therefore the window) has been realised. As a
belt-and-braces change, also add a check to the update_operations to prevent it
opening the popover if the window isn't yet realised.

(In reply to Carlos Soriano from comment #22)
> something, and then the popover dissapear, and not only that, the button
> dissapear too losing the ability to check it again. Then the user is angry :)
Completely agree. BTW, I wasn't talking about removing/hiding the button in any of the proposed changes - the behaviour or that would be unchanged. 

The patch I've just submitted is purely to fix the bug. I'll submit a separate patch to change the popover to open automatically only in the active window. It's a one-liner, but want to test it a bit more.

(In reply to Neil Herald from comment #24)
> Created attachment 325885 [details] [review] [review]
> toolbar: fix broken operations popover when new windows are opened
> 
> Opening another Nautilus window while a file operation is in progress causes
> the the operations popover to open in a broken state. The associated window
> isn't realised at the time the popover is first shown, meaning that the
> popover
> gets it's own window/title bar, which looks odd and doesn't work like it
> should.
> 
> This fix is to delay the initialisation of the operations popover and button
> until the toolbar (and therefore the window) has been realised. As a
> belt-and-braces change, also add a check to the update_operations to prevent
> it
> opening the popover if the window isn't yet realised.

Ok, I tried the patch. opening a new window while a file operation is in progress results in a new working window and the popover disappears from both windows after the file operation is over.

(In reply to Neil Herald from comment #25)
> The patch I've just submitted is purely to fix the bug. I'll submit a
> separate patch to change the popover to open automatically only in the
> active window. It's a one-liner, but want to test it a bit more.

How would that work? What would happen if I close the active window? Will the popup move to the second nautilus window or will the file operation be interrupted?

Created attachment 325894 [details] [review]
toolbar: fix broken operations popover when new windows are opened

Opening another Nautilus window while a file operation is in progress
causes the the operations popover to open in a broken state. It gets
it's own window/title bar, which looks odd and doesn't work correctly.

This fix delays the initialisation of the operations popover and button
until the UI elements have been realised. Also add a check to
update_operations to prevent it opening the popover if the window hasn't
been realised.

I've re-attached that patch - the commit message of my earlier one wasn't formatted correctly. Please ignore the earlier one

(In reply to Hussam Al-Tayeb from comment #26)
> Ok, I tried the patch. opening a new window while a file operation is in
> progress results in a new working window and the popover disappears from
> both windows after the file operation is over.
Thanks for checking it.

(In reply to Hussam Al-Tayeb from comment #27)
> How would that work? What would happen if I close the active window? Will
> the popup move to the second nautilus window or will the file operation be
> interrupted?
The popovers don't work like that. Regardless of which window you look at the popover, you'll see the same list - which is why I think it's redundant that it gets opened automatically in every window. If you close one of the windows, the operation will still run in the background and be shown in the list of the other windows. I'll submit the patch tonight, if you don't think it's right we can just leave it.

Review of attachment 325894 [details] [review]:

Thanks for digging into it!

The patch makes me think it's a gtk+ issue then... imho we shouldn't need to check the window state to set the widgets visible or not, or the buttons active or not.
I will check with gtk+ developers.

As discussed with Carlos Garnacho, this seems to be a gtk+ issue. Reassigning.

Fixed now in gtk+ master.

The following fix has been pushed:
46cdb44 GtkWindow: Ensure the toplevel is realized before realizing popovers

Created attachment 325987 [details] [review]
GtkWindow: Ensure the toplevel is realized before realizing popovers

Otherwise those get a NULL parent window, which is toplevel-y enough
to disembody the popover.

Can we have it on 3.20 too? thanks

It would be nice to have this in 3.20 as well so I don't have to remember to apply the patch when I do builds from gtk-3-20 branch ;)

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

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

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

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

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

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