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 734316 - CTRL + F doesn't toggle the search box‏
CTRL + F doesn't toggle the search box‏
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
3.12.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
review
Depends on:
Blocks:
 
 
Reported: 2014-08-05 20:17 UTC by Gianluca Sartori
Modified: 2015-03-04 14:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ephy-find-toolbar: Toggle the search bar with Ctrl+F (2.12 KB, patch)
2014-08-06 15:14 UTC, Yosef Or Boczko
none Details | Review
ephy-find-toolbar: Toggle the search bar with Ctrl+F (1.93 KB, patch)
2015-03-03 20:51 UTC, Yosef Or Boczko
committed Details | Review

Description Gianluca Sartori 2014-08-05 20:17:05 UTC
Using the shortcut CTRL + F shows the search box.
Using again the same shortcut does nothing -> should hide the search box.
Comment 1 Yosef Or Boczko 2014-08-05 20:50:45 UTC
What the version of epiphany?
It happen also when the keywaord layout is English?
Comment 2 Gianluca Sartori 2014-08-06 07:57:24 UTC
Epiphany 3.12.2

I didn't try with english keybord layout, I'll let you know ASAP. I'm using the Italian Kyb layout with English localization if it can help.
Comment 3 Yosef Or Boczko 2014-08-06 08:02:11 UTC
OK, this is what I wants to hear.
There is a problem with GtkAction and accels in non-LAtin (or non-English?) keyword
layout. in Hebrew I have the same problem.

We needs to port all the accels from handy accel and GtkAction accels to the new API
in GLib/GTK+ - GAction, it needs to solve this problem.
Comment 4 Michael Catanzaro 2014-08-06 14:31:14 UTC
(In reply to comment #3)
> We needs to port all the accels from handy accel and GtkAction accels to the
> new API
> in GLib/GTK+ - GAction, it needs to solve this problem.

We do need to do this, but I'm not sure it's related.  Ctrl+F does not toggle the search box in English either.
Comment 5 Yosef Or Boczko 2014-08-06 14:48:02 UTC
Oh, sorry, it was misunderstood!
I understand it not show the search bar  when hit Ctrl+F, you talking
on toggle, let me fix this.
Comment 6 Yosef Or Boczko 2014-08-06 15:14:22 UTC
Created attachment 282712 [details] [review]
ephy-find-toolbar: Toggle the search bar with Ctrl+F

If the search bar are shown and the entry are focus,
Ctrl+F will hide the search bar. If the search bar
are shown but not focused, the entry will get focus.
Otherwise Ctrl+F will show the search bar and than
focus the search entry.
Comment 7 Michael Catanzaro 2015-02-26 20:57:10 UTC
Review of attachment 282712 [details] [review]:

This works nicely!
Comment 8 Yosef Or Boczko 2015-02-26 23:14:09 UTC
I think it intended to ;-)
To push?
Comment 9 Michael Catanzaro 2015-02-27 03:15:25 UTC
Wait for a maintainer please.
Comment 10 Carlos Garcia Campos 2015-02-27 06:42:41 UTC
Review of attachment 282712 [details] [review]:

::: embed/ephy-find-toolbar.c
@@ +716,3 @@
+
+void
+ephy_find_toolbar_toggle_mode (EphyFindToolbar *toolbar)

I would call this just toggle or toggle_state

@@ +720,3 @@
+	EphyFindToolbarPrivate *priv;
+
+	g_return_if_fail (EPHY_IS_FIND_TOOLBAR (toolbar));

This is actually a private method, don't use g_return macros

@@ +724,3 @@
+	priv = toolbar->priv;
+
+	if (gtk_widget_is_focus (priv->entry))

Is this enough/accurate to know if the findbar is visible? if you have clicked up/down buttons, the focus is moved to the buttons (I think that's indeed another bug, those buttons should have focus on click set to FALSE, but still).
Comment 11 Yosef Or Boczko 2015-02-27 14:12:34 UTC
(In reply to Carlos Garcia Campos from comment #10)
> Review of attachment 282712 [details] [review] [review]:
> 
> ::: embed/ephy-find-toolbar.c
> @@ +716,3 @@
> +
> +void
> +ephy_find_toolbar_toggle_mode (EphyFindToolbar *toolbar)
> 
> I would call this just toggle or toggle_state

toggle_state sound good to me.

> 
> @@ +724,3 @@
> +	priv = toolbar->priv;
> +
> +	if (gtk_widget_is_focus (priv->entry))
> 
> Is this enough/accurate to know if the findbar is visible? if you have
> clicked up/down buttons, the focus is moved to the buttons (I think that's
> indeed another bug, those buttons should have focus on click set to FALSE,
> but still).

Read the commit message, it tuned:
„If the search bar are shown but not focused, the entry will get focus.”

If you wants Ctrl+F to hide the search bar in this case too, I can to check
by gtk_search_bar_get_search_mode (toolbar) instead by gtk_widget_is_focus (entry).
Comment 12 Michael Catanzaro 2015-02-28 01:06:03 UTC
I like the current behavior.

(In reply to Yosef Or Boczko from comment #11) 
> toggle_state sound good to me.

Carlos, is this good with the function renamed accordingly?
Comment 13 Carlos Garcia Campos 2015-02-28 08:32:36 UTC
(In reply to Michael Catanzaro from comment #12)
> I like the current behavior.
> 
> (In reply to Yosef Or Boczko from comment #11) 
> > toggle_state sound good to me.
> 
> Carlos, is this good with the function renamed accordingly?

I don't think it's correct to only toggle the find bar visibility when the focus is in the entry.
Comment 14 Yosef Or Boczko 2015-03-03 20:51:40 UTC
Created attachment 298483 [details] [review]
ephy-find-toolbar: Toggle the search bar with Ctrl+F
Comment 15 Michael Catanzaro 2015-03-04 14:40:45 UTC
Attachment 298483 [details] pushed as 085f73a - ephy-find-toolbar: Toggle the search bar with Ctrl+F