Bug confirmed it only happens with "\" char but not with "/". 

Ok, this is way trickier than it seems at first: the text that is inserted in the entry (either by typing, pasting or by selecting an item in the history) needs to be escaped.
The problem descibed is due to the fact that we story in the history the escaped text, so when an item is selected from the history it is escaped twice.

We have two alternatives:
1 - detect that the text comes from the history and in that case we do not escape it
2 - unescape the text before storing it in the history


I can't see any way to do (1) so I tried (2), but the problem is that the history completion popup would at this point display the unescaped text, which is particularly bad if a \n is there since it gets split in two lines.

My idea was to set a data_func on the cell renderer of the combo so that we could escape the displayed text on the fly while still storing in the history the unescaped text. Unfortunately I cannot find a way to get to such cell renderer... I am attaching my patch (which doesn't work) as a reference. If anyone has ideas how to do that please let me know.


> it only happens with "\" char but not with "/". 

Thats totally expected \ is the character used to escape \n, \t etc

Created attachment 95112 [details] [review]
non working patch

Created attachment 95116 [details] [review]
patch

Actually it turns out that the problem with the previous patch was a bug in gtk (combo didn't implement the get_cells method).
The patch was also missing an unescape call when opening the dialog.

With this patch everything seems to work properly here, but review and testing are appreciated.

Also the patch requires gtk from svn trunk, so I am not sure when if we can commit this in time for feature freeze...

Paolo i can help you as soon as i get and build gtk from trunk. But it might be too late for feature freeze.

Does this patch still depend on svn trunk?

Comment on attachment 95116 [details] [review]
patch

Patch looks good to me, but I have not tested it.

Comments:

> 		g_free (data->find_text);
>-		data->find_text = g_strdup (str);
>+		data->find_text = gedit_utils_unescape_search_text (str);

Are we sure gedit_utils_unescape_search_text is the exact inverse function of gedit_utils_escape_search_text and so
that we alway have 

  text == unescape(escape(text))

If this is not true, the above code could be wrong (even if we probably have similar problem in other places too).

> 	}
> 
> 	str = gedit_search_dialog_get_replace_text (dialog);
> 	if (str != NULL && *str != '\0')
> 	{
> 		g_free (data->replace_text);
>-		data->replace_text = g_strdup (str);
>+		data->replace_text = gedit_utils_unescape_search_text (str);

Same as above.

> 	}
> 
> 	data->match_case = gedit_search_dialog_get_match_case (dialog);
>Index: gedit/dialogs/gedit-open-location-dialog.c
>===================================================================
>--- gedit/dialogs/gedit-open-location-dialog.c	(revision 5886)
>+++ gedit/dialogs/gedit-open-location-dialog.c	(working copy)
>@@ -101,9 +101,12 @@ response_handler (GeditOpenLocationDialo
> 
> 				text = gtk_entry_get_text
> 						(GTK_ENTRY (dlg->priv->uri_text_entry));
>-				gedit_history_entry_prepend_text
>-						 (GEDIT_HISTORY_ENTRY (dlg->priv->uri_entry),
>-						  text);
>+				if (*text != '\0')
>+				{
>+					gedit_history_entry_prepend_text
>+							 (GEDIT_HISTORY_ENTRY (dlg->priv->uri_entry),
>+							  text);
>+				}

This seems ok.
But just to be sure I have understood correctly the problem: this is unrelated to the problem we had, right?

			
>-	if (g_utf8_strlen (text, -1) < MIN_SEARCH_COMPLETION_KEY_LEN)
>+
>+	text = gedit_utils_unescape_search_text (str);
>+
>+	if (g_utf8_strlen (str, -1) < MIN_SEARCH_COMPLETION_KEY_LEN)
>+	{
>+		g_free (text);
> 		return;
>-	
>+	}

hmmm... there is something wrong here.
Why are you first computing "text" and then check for the lenght of "str"?

May be you should compute "text" only after the "if" block.

> 		if (strcmp (text, str_data) == 0)
> 		{
>+			g_free (text);
> 			g_free (str_data);
> 			gtk_list_store_move_after (GTK_LIST_STORE (model),
> 						   &iter,

I'm lazy and so I'm not checking the current code... are you sure "text" will not be used anymore?

> Does this patch still depend on svn trunk?

No the fix went in gtk 2.12.0. I bumped the reqs accordingly.

> Are we sure gedit_utils_unescape_search_text is the exact inverse function of
> gedit_utils_escape_search_text

It is as far as I can see... if it isn't it is a bug.

> This seems ok.
> But just to be sure I have understood correctly the problem: this is unrelated
> to the problem we had, right?

Yeah... I spotted this buglet while grepping. It is unrelated and I forgot about it

> hmmm... there is something wrong here.
> Why are you first computing "text" and then check for the lenght of "str"?

You are right. I changed to check the lenght of the computed text.

> I'm lazy and so I'm not checking the current code... are you sure "text" will
> not be used anymore?

Yes, there is a 'return' on the next line