Opened 10 years ago

Last modified 10 years ago

#8799 new defect

Small adjustment to previous fix in Message Notification Plugin

Reported by: Luigiwalser Owned by: deryni
Milestone: Component: pidgin (gtk)
Version: 2.5.5 Keywords:
Cc:

Description

This is an addendum to Bug 2145, which I would have reopened, but I see no way to do that. http://developer.pidgin.im/ticket/2145

The patch I had posted there worked perfectly. The patch that was accepted (also attached there) caused me a problem that I had to turn focus stealing prevention all the way up on my window manager to resolve, and even that wasn't a perfect resolution.

I didn't occur to me why the accepted patch was behaving differently until now. My patch had it show/present then raise, the accepted patch is in the other order.

Could you please switch the order of these two if blocks in notify.c to fix it? Thank you.

if (purple_prefs_get_bool("/plugins/gtk/X11/notify/method_raise"))

handle_raise(purplewin);

if (purple_prefs_get_bool("/plugins/gtk/X11/notify/method_present"))

handle_present(conv);

Change History (12)

comment:1 Changed 10 years ago by deryni

  • Status changed from new to pending

What problem is it causing you?

It would seem to me that the only problem with the current ordering is that the window may not be raised to the top of the Z-order if it was minimized when the message was received (and the window manager interprets the present message as deiconify-to-previous-location-and-z-index).

comment:2 Changed 10 years ago by deryni

  • Owner changed from lschiere to deryni

comment:3 Changed 10 years ago by Luigiwalser

  • Status changed from pending to new

The problem was this. If the window was not minimized, it will be raised, but not take focus, which is the desired behavior. If the window was minimized however, it would both be deiconified AND steal focus, which is bad. My patch didn't cause this behavior, so I figure the raise/present order must be the issue, as it's essentially the only difference between the two patches.

comment:4 Changed 10 years ago by lschiere

  • Component changed from unclassified to pidgin (gtk)

comment:5 Changed 10 years ago by deryni

  • Status changed from new to pending

Sorry for taking so long to get back to this, if you switch the order of those two blocks in the current plugin do you stop getting your undesirable behaviour? Assuming you are still seeing that behaviour.

It seems more likely to me that the issue is that your patch used gdk_window_show directly and that the accepted patch uses (eventually) gtk_window_present. gtk_window_present will call gdk_window_show but will also call gdk_window_focus (which, for window management environments that support the EWMH will request that the window be made active and for those that don't will call XRaiseWindow).

"Raise but not take focus" when the window is unminimized is what the 'Raise' option does, purely unminimize is not currently an option, "do what is needed to get attention" is what the 'Present' option does currently.

I'm hesitant to add yet another option for 'Unminimize' but I'm not sure how else to actually solve this issue, given that I'm reasonably sure some people are going to be unhappy no matter which way 'Present' actually works (focusing or not focusing the window).

comment:6 Changed 10 years ago by trac-robot

  • Status changed from pending to closed

This ticket was closed automatically by the system. It was previously set to a Pending status and hasn't been updated within 14 days.

comment:7 Changed 10 years ago by Luigiwalser

It appears the raise option is not working correctly. After switching the two if blocks, this is the behavior I am getting. With focus stealing prevention turned off, it raises and focuses. With focus stealing prevention turned on, it neither raises nor focuses. The present option to unminimize works fine with focus stealing prevention on, but steals focus if it's not.

This is really frustrating. It shouldn't be this difficult to get correct behavior (make the window visible so you can see it when someone sends a message, no matter the window's current status; never steal focus). If there's anything else I can do to help get this fixed, let me know.

Sorry it took so long to respond, I didn't have time to test it. BTW, can you fix Trac so bugs can be reopened? Thanks.

comment:8 Changed 10 years ago by deryni

  • Status changed from closed to new

comment:9 Changed 10 years ago by deryni

  • Status changed from new to pending

The issue here is that it really is difficult to cover all possible desired outcomes.

Wanting the window to get focus when a message comes in is perfectly reasonable (though I don't understand it).

Wanting the window to be raised when the message comes in is perfectly reasonable.

Wanting "raise" to include unminimizing is reasonable. *Not* wanting "raise" to include unminimizing is reasonable.

Further complicating things is the issue of how multiple desktops should be handled (luckily this isn't something pidgin needs to deal with directly, but is something *users* need to deal with and something that the major desktops/window managers tend to handle terribly and with a decided lack of user control/input).

At the moment pidgin allows the user to say:

"I want the window to raise" which means pidgin will call gdk_window_raise (what that does offhand I'm not sure)

and

"I want the window to be shown" which means pidgin will make sure the conversation is in a conversation window (not hidden to the buddy list, etc.) and call gtk_window_present (what that does offhand I'm not sure).

The important thing to note here is that pidgin does not explicitely request focus (if GTK+ does as a result of gtk_window_present that is unfortunate but not something we are requesting specifically).

When you were describing your current behaviour the first two were with just the "Raise" option on and the latter was with just the "Present" option on?

Can you get a full matrix of focus prevention on/off against each single option on/off, both options on/off (and ideally both orders of the if blocks)? I realize that's a lot of work so feel free to say no.

comment:10 Changed 10 years ago by Luigiwalser

  • Status changed from pending to new

I'll do any tests you want, including those. I won't be able to do the tests until next week, but it's no problem. Just for the record, last time I posted I was testing with both Raise and Present enabled.

comment:11 Changed 10 years ago by Luigiwalser

I looked at the old comments and code as I'm preparing to do these tests, and I have another idea about what might be the problem. I don't know if the order of the if blocks matters, but handle_present should be calling pidgin_conv_window_show (which is what my suggestion in bug 2145 which worked correctly did) and not purple_conversation_present (which I have no idea what that does). Actually it sounds like you may have already figured that out from your comments on this bug.

comment:12 Changed 10 years ago by Luigiwalser

OK, with focus prevention off, the raise option does what you'd expect. It raises only, it doesn't steal focus or unminimize. The present option raises, steals focus, and unminimizes. It should only unminimize. In this case, I don't think the if block order matters.

With focus prevention on, the unminimizing works (good), there's no focus stealing (good), but raising doesn't happen with either option (bad).

The present option was added for unminimization in bug 2145, but I didn't complain about the focus stealing right away after it was added, because I didn't know the reason for it. The way you describe the present option, it sounds like you're thinking about trying to work around the same metacity bug that caused Mandriva to add the stupid broken patch that caused bug 8780 to be filed. So neither is solving that problem well, and I'm not sure either Pidgin or Mandriva (via a patch to Pidgin) should be trying to, but that's another argument :o).

I tried changing handle_present to call pidgin_conv_window_show and that didn't work, it's just not unminimizing, which really blows my mind. I don't understand why that won't work now.

Let me know if you want anything else from me.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!