Created attachment 38674 [details] [review]
patch for gnome-panel/gnome-panel-screenshot.c and panel-run-dialog.c

Attached the patch.

Takao: thanks for the patch.

Here are some comments:

 + the screenshot part of the patch should go to gnome-utils since we moved the
code there

 + I'm not sure about this:
       result = panel_run_dialog_show_url (dialog, url_locale, escaped_locale);
   I don't think we should use escaped_locale since it's only used to display an
error message. I'm also not sure about url_locale: does gnome_vfs_show_uri()
need a non UTF-8 string?

 + also, the gtk_im_context_filter_keypress() should probably be at the
beginning of entry_event, shouldn't it?

Created attachment 39595 [details] [review]
patch for panel-run-dialog.c

Revised patch 38674

Thanks for your comments All comments are right so I revised the patch.
I'll file another bug for screenshot app.

Comment on attachment 39595 [details] [review]
patch for panel-run-dialog.c

Small comment on the entry_event part of the patch patch:

>+	/* commit the string with Enter key */
>+	if (GTK_ENTRY (entry)->editable) {
>+    		if (gtk_im_context_filter_keypress (GTK_ENTRY (entry)->im_context, event)) {

Is it okay to use GTK_ENTRY (entry)->im_context? It's marked as private in
gtkentry.h. But I can't find any other way to do this...

>+			return TRUE;

This should probably return FALSE (we did not handle the keypress ourselves).

>+		}
>+	}

I'm just worried about the im_context that is private. I'll commit your patch
if you are sure there's no other way to do this or if you know it's okay to do
this :-)

> Is it okay to use GTK_ENTRY (entry)->im_context? It's marked as private in
gtkentry.h. But I can't find any other way to do this...

I don't have other ways to get im_context. I think it's ok.

> This should probably return FALSE (we did not handle the keypress ourselves).

gtk_im_context_filter_keypress will commit the preedit string so need to return
TRUE. I copied the comment in gtkimcontext.c:

/**
 * gtk_im_context_filter_keypress:
 * @context: a #GtkIMContext
 * @event: the key event
 *
 * Allow an input method to internally handle a key press event.
 * If this function returns %TRUE, then no further processing
 * should be done for this keystroke.
 *
 * Return value: %TRUE if the input method handled the keystroke.
 *
 **/

So it's ok to return TRUE in this API.

Takao: thanks for your answers!

I just tested the patch again and it seems to break the autocompletion here:
gtk_im_context_filter_keypress() always returns TRUE. I'm not using any IM and
I'm just typing some ascii text.

Does this happen for you too? Do you have any idea on how to fix this?

I'm wondering if we really need to call gtk_im_context_filter_keypress() here:
it's already called in gtk_entry_key_press().

(I'll probably commit the first part of the patch soon)

Created attachment 39615 [details] [review]
patch for panel-run-dialog.c

Reverted patch 38674.

Vincent, 

Sorry for confusing you. I found some problems in the second patch so reverted
the the original patch. 

> Does this happen for you too? Do you have any idea on how to fix this?

I could not reproduce it. Could you try if your problem is happened with the
third patch. If you can, could you give me more information about the problem?
I checked the gconf value /apps/gnome-settings/gnome-panel is saved correctly.

I remembered the original bug. Yes, gtk_im_context_filter_keypress is needed. 
If the api is missed, I cannot input multibyte chars in the first column. I
think  the behavior is overrided by gtk_entry_set_text in gtk_entry.

I filed bug 172435 for gnome-panel-screenshot.

Takao: the first part of the patch is committed.
What I don't understand is why the call to gtk_im_context_filter_keypress()
doesn't work at the beginning of entry_event(). If it doesn't work there, then
it surely breaks things to call it later in the function, doesn't it?

Created attachment 45432 [details] [review]
patch for panel-run-dialog.c

Revised patch 39595.

Vincent,

Sorry for my late response. I was on a biz trip last week.
Yes, your mention is right and I noticed the problem at the beginning of
entry_event(). I revised the patch. It's ok with me.

Vincent, should we apply the rest of this fix now or do you still have issues
with it?

Vincent, just following up on this patch, the last secton of the patch to
panel-run-dialog.c in : entry_event(), can this be applyed or is there
still isses ?

The entry_event problem exists in 2.16.1
Could you integrate the fix for gtk_im_context_filter_keypress?

Sorry for the late reply. I tried again, and it still breaks autocompletion for me.

It looks to me like there's no easy way to fix this. So what about finding another way to do this? We save the string we have in the entry before the event, and we compare with what we have after the im handling. This way we can know if the string changed and update the selection accordingly.

Created attachment 81978 [details] [review]
Patch for panel-run-dialog.c with the request

I don't know what is your problem in the autocompletion.
Could you explain how to reproduce your problem?

I'm also attaching the different patch which don't return the function soon.
What about this patch?

Is this still an issue in a correct GNOME version, like 2.28?

Yes, I still can reproduce this problem in 2.28.

bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years.

If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/gnome-panel/-/issues/

Thank you for reporting this issue and we are sorry it could not be fixed.