Moving to 2.20 target due to feature and UI freeze for 2.18.

Moving Severity:enhancement bugs off of 2.20 target.

I'm interested in implementing this, be it in an extension (as discussed in another bugreport) or in Epiphany itself, at least if I'm given a go. :)

Setting the tooltip itself should work now with the new gtk tooltips system; just use gtk_tool_item_set_tooltip_text(). You can get the title from ephy_embed_shistory_get_nth().

(However middle ellipsisation isn't possible with the gtk tooltips framework.)

Alright. Now I'm wondering, after having had a look at src/ephy-tab.c, whether the appropriate would be in “ephy_tab_update_navigation_flags()”, since it seems to be called from several places, and that's where I would tend to put such tooltip text update code.

I discovered “sync_tab_navigation()” in src/ephy-window.c some minutes later, but I'm still patching. I'll keep you posted in less than some hours, I hope. :)

Some comments on the following patch:
 - I played a bit with “array-tooltip” before “tooltip”, that's why this patch
   isn't using “gtk_tool_item_set_tooltip_text()”; AFAIUI, that allows the
   tooltip text to be also displayed in the statusbar, that's why I'm not sure
   it is needed that I rewrite it to use the function you mentioned, but of
   course, I may be totally wrong;
 - that's the first time I play with g_value*, I'm not sure I did it the right
   way;
 - I was first tempted to also pass a “up_title” paramter (see the
   “ephy_toolbar_set_navigation_actions()” prototype, but I can't see how we
   could get its *title* (the URL could be computed, though), that's why I
   dropped it.

As always, comments and slapping welcome. :)

Created attachment 95201 [details] [review]
Add back/forward titles to navigation tooltips

Thanks for the patch!

+	g_value_set_string (&value, back_title);
+	g_object_set_property (G_OBJECT (priv->actions[BACK_ACTION]),
+			       "tooltip", &value);
+
+	g_value_set_string (&value, forward_title);

You can use g_value_set_static_string to save string copies.

+	g_value_set_string (&value, forward_title);
+	g_object_set_property (G_OBJECT (priv->actions[FORWARD_ACTION]),
+			       "tooltip", &value);
+}

You need g_value_unset (&value) afterwards here to clean up.

+	if (ephy_embed_shistory_get_pos (embed) > 0)
+	{
+		ephy_embed_shistory_get_nth (embed, -1, TRUE,
+					     &back_url, &back_title);
+	}
+
+	if (ephy_embed_shistory_get_pos (embed) < ephy_embed_shistory_n_items (embed) - 1)
+	{

Just store the _get_pos result in a local variable instead of calling it twice. And use "... else if (...)" here.

+	ephy_toolbar_set_navigation_tooltips (window->priv->toolbar,
+					      back_title,
+					      forward_title);

You need to free back/forward_title and _url.

Actually maybe the implementation of ephy_embed_shistory_get_nth should be changed to allow passing NULL if you're not interested in the URL, so we make fewer string copies here.

(Note that due to bug 458283 the tooltips might not work correctly right now; but that shouldn't stop a patch from being committed to epiphany.)

Created attachment 95229 [details] [review]
Updated navigation tooltips patch

 - g_value_set_static_string: Done, thanks.
 - g_value_unset: Done, thanks.
 - position: Done, I wasn't sure just calling it twice would imply
   the need of caching this value, but OK, I'm learning. :)
 - if/else if: discussed on IRC, not done.
 - g_free: usually I try adding them, and see whether they are needed
   (SEGV otherwise). Maybe you would accept a documentation patch, so
   that it is specified that they are malloc()'d / have to be freed?
 - I agree that its accepting NULL for parameters I don't care about
   is what I expected in the first place.

Let's do a follow-up patch in a new bug for the NULL parameters then.

Thanks for the patch! oktc, trunk only.

Committed the patch. Closing this bug as FIXED, as for comment #10.

2007-09-10  Cosimo Cecchi  <cosimoc@svn.gnome.org>

        * src/ephy-toolbar.c: (ephy_toolbar_set_navigation_tooltips):
        * src/ephy-toolbar.h:
        * src/ephy-window.c: (sync_tab_navigation):

        Makes tooltips for Back and Forward buttons display the titles of
        relative pages. Fixes bug #341400. Patch by Cyril Brulebois.