After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 363848 - Can't select a different bookmark topic by moving the mouse
Can't select a different bookmark topic by moving the mouse
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Bookmarks
git master
Other Linux
: Normal normal
: gnome-2-20
Assigned To: Epiphany Maintainers
Marco Pesenti Gritti
Depends on:
Blocks:
 
 
Reported: 2006-10-21 06:15 UTC by John Millikin
Modified: 2007-04-02 20:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Select topics by moving the mouse (1.52 KB, patch)
2006-10-21 06:16 UTC, John Millikin
rejected Details | Review
Select topics by moving the mouse (2.30 KB, patch)
2007-03-05 21:01 UTC, John Millikin
none Details | Review
Select topics by moving the mouse, r3 (2.33 KB, patch)
2007-03-06 18:24 UTC, John Millikin
reviewed Details | Review
Select topics by moving the mouse, r4 (2.95 KB, patch)
2007-03-28 01:41 UTC, John Millikin
committed Details | Review

Description John Millikin 2006-10-21 06:15:32 UTC
In most other browsers, once the user has selected a bookmark topic/folder by clicking on it in the toolbar, they're able to move the mouse to a different topic and have its menu pop up. Epiphany doesn't support this, the user must click on the new topic to select it (annoying).
Comment 1 John Millikin 2006-10-21 06:16:54 UTC
Created attachment 75121 [details] [review]
Select topics by moving the mouse

Attached patch, but I don't know how GNOME-ish it is. Might need cleanup.
Comment 2 Christian Persch 2006-11-03 19:49:28 UTC
Thanks for the patch.

First, I'm not sure this is really expected behaviour; a toolbar is not a menubar after all.

Still here's a comment on the patch:

+	static GtkWidget* activated_button = NULL;
+	static EphyTopicAction* activated_action = NULL;
[...]
+	activated_button = button;
+	activated_action = action;

No, no, no. :) Using static variables is just wrong.
Comment 3 John Millikin 2006-11-03 23:28:22 UTC
I can't find any other gnome applications that use a toolbar with drop-down menus, but the expected behaviour in web browsers is indeed to treat bookmark tool bars as menu bars.

As for the patch, yes it is very ugly; the activated_* variables should be attached to the bookmark list, though I have no idea how to do that.
Comment 4 Reinout van Schouwen 2006-11-04 16:19:16 UTC
I agree with John, it would be nice if topic dropdowns would open automatically if you move the mouse cursor to it from an open neighbouring topic.

Even if we won't do this, it's wrong that opening a different topic requires two mouse clicks, one to close the open topic and one to open the second one.
Comment 5 John Millikin 2007-03-05 21:01:44 UTC
Created attachment 84005 [details] [review]
Select topics by moving the mouse

Now with 100% less 'static'
Comment 6 Christian Persch 2007-03-06 18:09:41 UTC
harves, could you take a look to see if this is correct? It looks ok to me, apart from the codestyle issues (commented on IRC about these).
Comment 7 John Millikin 2007-03-06 18:24:56 UTC
Created attachment 84103 [details] [review]
Select topics by moving the mouse, r3

Same code as the last patch, with correct coding style
Comment 8 Peter Harvey 2007-03-17 20:24:14 UTC
Looks good to commit as far as I'm concerned.

There are some minor things that could be changed, like finding a way to avoid recording the action; just recording the button should be sufficient, and button_toggle_cb might be the best place to call erase_popup, but I'm not sure, and none of it is necessary. Nice work. :)
Comment 9 Christian Persch 2007-03-27 19:29:26 UTC
Nice effect :)

+               g_object_set_data (G_OBJECT (window),
+                                  "active-topic-action-button",
+                                  button);
+               g_object_set_data (G_OBJECT (window),
+                                  "active-topic-action",
+                                  action);
[...]
+               g_object_set_data (G_OBJECT (window),
+                                  "active-topic-action-button", NULL);
+               g_object_set_data (G_OBJECT (window),
+                                  "active-topic-action", NULL);
[...]

It's enough to store the button, since you can get the action with gtk_widget_get_action from it.

I'm a bit uncomfortable with unsetting the data on button_toggled_cb, since there are instances where the menu is deactivated but the button not unpressed, e.g. when we get a grab shadowing or another tab tries to steal the focus. (That's a bug, but I'd rather be safe here).
Comment 10 John Millikin 2007-03-27 19:59:52 UTC
Actually, calling gtk_widget_get_action on the active button always returns NULL. Is the action is supposed to be recorded along with the button? If so, I could open another bug.

By unsetting the data in button_toggled_cb, I hoped to avoid any situation where active-topic-action is non-NULL but no menu exists. I figure it's better to have a button stuck down, which is current behaviour in the face of such an error.
Comment 11 Christian Persch 2007-03-27 20:52:02 UTC
I realised that the get_action returns NULL because the toolbar items aren't created by gtkuimanager.

Yes, but if the button is stuck down there won't have been a toggled signal (else it would be 'up') and thus the old action is still set as data on the window. If the action is now destroyed (e.g. by removing the topic), there might be a crash...
Comment 12 John Millikin 2007-03-28 01:41:55 UTC
Created attachment 85418 [details] [review]
Select topics by moving the mouse, r4

This patch sets the data "gtk-action" on toolbar buttons, removing the need to store the action separately. It also moves unsetting "active-topic-action-button" into button_deactivate_cb, which is called upon the menu popup's "deactivate" signal.
Comment 13 Christian Persch 2007-04-01 22:00:20 UTC
Looks fine to me now.
Comment 14 Diego Escalante Urrelo (not reading bugmail) 2007-04-02 20:36:42 UTC
Thanks John!. Committed to trunk, rev 6981.

2007-04-02  Diego Escalante Urrelo <diegoe@gnome.org>

        * src/bookmarks/ephy-topic-action.c:

        Allow the user to select a different bookmark topic on the toolbar by
        just moving the mouse. Old behaviour forced the user to click each
        topic button to activate the menu, now only the first click is
        required. Bug #363848. Patch by John Millikin.