*** Bug 675054 has been marked as a duplicate of this bug. ***

Created attachment 304585 [details] [review]
ephy-embed-utils: add ephy_embed_utils_autosearch_address()

Factor the code to handle a search key from
ephy_embed_utils_normalize_or_autosearch_address() to a new method to
be able to reuse it later.

Created attachment 304586 [details] [review]
ephy-web-dom-utils: add ephy_web_dom_utils_get_selection_as_string()

The DOMSelection::toString() API is only bound to JavaScript. This method
does not work with selection inside input elements.

Created attachment 304587 [details] [review]
Add context-sensitive menu option to search the web for selected text

When building the context menu in the web process, use the web
extension to find out whether there is text selected and add a custom
menu item with the selected text. This menu item will be used as the
basis to build a new menu item in the UI process. When activated, this
menu item will launch a new tab and search the selected text in the
user-preferred search engine.

Review of attachment 304586 [details] [review]:

::: lib/ephy-web-dom-utils.c
@@ +495,3 @@
+
+char*
+ephy_web_dom_utils_get_selection_as_string   (WebKitDOMDOMSelection *selection)

Some funky formatting here; missing space between the char and the *, and three spaces before the opening parenthesis?

Review of attachment 304585 [details] [review]:

Ok

Review of attachment 304586 [details] [review]:

We could probably make WebKit expose toString(), unless there's a good reason to be JS only. Please, fix the formatting issues and the leak before landing.

::: lib/ephy-web-dom-utils.c
@@ +498,3 @@
+{
+  WebKitDOMRange *range = webkit_dom_dom_selection_get_range_at (selection, 0, NULL);
+  return range ? webkit_dom_range_to_string (range, NULL) : NULL;

This is leaking the range, you need to g_object_unref it after to_string().

Review of attachment 304587 [details] [review]:

Since we are building the action in the UI process in the end, I think we could avoid all the complexity by simply passing the selected text as user data. That way it could also be used to build other context menu items based on the selected text in the end. And if we eventually add the selected text to WebKitHitTestResult, most of the code will be the same.

::: embed/web-extension/ephy-web-extension.c
@@ +42,3 @@
+#define WEBKIT_DOM_USE_UNSTABLE_API
+#include <webkitdom/WebKitDOMDOMWindowUnstable.h>
+#include <webkitdom/webkitdom.h>

You are including this twice.

@@ +1010,3 @@
+  WebKitDOMDOMSelection *selection = webkit_dom_dom_window_get_selection (window);
+
+  string = ephy_web_dom_utils_get_selection_as_string (selection);

selection can also be NULL. Note also that webkit_dom_document_get_default_view() and webkit_dom_dom_window_get_selection() are both transfer full, so you need to free the window and the selection.

@@ +1016,3 @@
+  if (*string == '\0')
+  {
+    g_free (string);

g_free is null safe, so you can use the same if

if (!string || !*string)
{
  g_free (string);
  g_object_unref (selection);
  g_object_unref (window);
  return FALSE;
}

::: src/ephy-window.c
@@ +1636,3 @@
 }
 
+static WebKitContextMenuItem*

static WebKitContextMenuItem *

@@ +1637,3 @@
 
+static WebKitContextMenuItem*
+find_custom_item_in_context_menu (WebKitContextMenu *context_menu, const char* action_name)

const char *action_name

@@ +1653,3 @@
+			 * extension for details.
+			 */
+			char** tokens = g_strsplit (gtk_action_get_label (action), "|", 2);

char **tokens

@@ +1665,3 @@
+}
+
+static char*

static char *

@@ +1754,3 @@
+	{
+		const char *string;
+		char* ellipsized;

char *ellipsized;

@@ +1763,3 @@
+		if (ellipsized)
+		{
+			char* label;

char *label;

Review of attachment 304586 [details] [review]:

It seems to me that toString() is a JS-specific thing; all objects have a toString() method that can be use to query the contents, and in the case of DOMSelection, the selection contents are returned.

Created attachment 304777 [details] [review]
ephy-embed-utils: add ephy_embed_utils_autosearch_address()

Factor the code to handle a search key from
ephy_embed_utils_normalize_or_autosearch_address() to a new method to
be able to reuse it later.

Created attachment 304778 [details] [review]
ephy-web-dom-utils: add ephy_web_dom_utils_get_selection_as_string()

The DOMSelection::toString() API is only bound to JavaScript. This method
does not work with selection inside input elements.

Created attachment 304779 [details] [review]
Add context-sensitive menu option to search the web for selected text

When building the context menu in the web process, use the web
extension to find out whether there is text selected and add a custom
menu item with the selected text. This menu item will be used as the
basis to build a new menu item in the UI process. When activated, this
menu item will launch a new tab and search the selected text in the
user-preferred search engine.

Review of attachment 304778 [details] [review]:

Fix my comments before landing, please.

::: lib/ephy-web-dom-utils.c
@@ +509,3 @@
+
+  string = range ? webkit_dom_range_to_string (range, NULL) : NULL;
+  g_object_unref (range);

g_object_unref is not null safe. Use an early return instead if (!range) return NULL;

::: lib/ephy-web-dom-utils.h
@@ +52,3 @@
                                                           long             *y);
+
+char * ephy_web_dom_utils_get_selection_as_string (WebKitDOMDOMSelection *selection);

char * ephy -> char *ephy

Review of attachment 304779 [details] [review]:

Ok. I'm fine with the patch, just consider my comments before landing.

::: embed/web-extension/ephy-web-extension.c
@@ +1008,3 @@
+  WebKitDOMDOMWindow *window = webkit_dom_document_get_default_view (document);
+  WebKitDOMDOMSelection *selection = webkit_dom_dom_window_get_selection (window);
+  g_object_unref (window);

Leave an empty line between the declaration block and the body.

@@ +1023,3 @@
+
+  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
+  g_variant_builder_add (&builder, "{sv}", "SearchSelection", g_variant_new_string (g_strstrip (string)));

Since the only user data we have is the string I would pass the string directly, instead of using a dictionary. When we need to pass more data we can switch to use a dictionary. But still, I'm fine with the dict now that you have done it. I would make it more generic and use "SelectedText" instead of "SearchSelection" for they key, though.

::: src/ephy-window.c
@@ +1634,3 @@
 
+static char *
+context_menu_ellipsize_string (const char *string, glong max_length)

This is actually quite generic, it does nothing specific to the context menu no? I would call it just ellipsize_string().

@@ +1721,3 @@
 
+	g_variant_dict_init (&dict, webkit_context_menu_get_user_data (context_menu));
+	if (g_variant_dict_lookup (&dict, "SearchSelection", "s", &selection_string))

I think you can use "&s" and get a const char *, instead. You can duplicate it later only when needed.

@@ +1734,3 @@
+			label = g_strdup_printf (_("Search the Web for '%s'"), ellipsized);
+			gtk_action_set_label (action, label);
+			g_object_set_data_full (G_OBJECT (action), "selection", selection_string,

Here you would g_strdup() it.

@@ +1741,3 @@
+		}
+		else
+			g_free (selection_string);

And you wouldn't need this branch.

@@ +1742,3 @@
+		else
+			g_free (selection_string);
+	}

I would move part of this to a helper function, something like parse_context_menu_user_data() that returns the selected_text as an out parameter.

Attachment 304777 [details] pushed as b92a519 - ephy-embed-utils: add ephy_embed_utils_autosearch_address()
Attachment 304778 [details] pushed as b516557 - ephy-web-dom-utils: add ephy_web_dom_utils_get_selection_as_string()
Attachment 304779 [details] pushed as 9fdf920 - Add context-sensitive menu option to search the web for selected text

*** Bug 724919 has been marked as a duplicate of this bug. ***

This is nice feature and I like it, but looks like it disappeared on my system, currently using epiphany-3.18.0-1.fc23.x86_64  it worked fine before with some dev releases 3.17.x, problem is, I cannot remember when it stopped working. Anyone else see this broken with final 3.18.x release(s)? (I can open new bug report for this if needed)

It's working for me in 3.18.0. That might be an issue with the web extension in the Fedora package. Please report to the Fedora developers.

(In reply to (bitlord) from comment #17)
> This is nice feature and I like it, but looks like it disappeared on my
> system, currently using epiphany-3.18.0-1.fc23.x86_64  it worked fine before
> with some dev releases 3.17.x, problem is, I cannot remember when it stopped
> working. Anyone else see this broken with final 3.18.x release(s)? (I can
> open new bug report for this if needed)

Do you see any warnings when starting the browser in the terminal? Does password saving work? What about the Load Anyway button if you visit https://self-signed.badssl.com/ ?

(In reply to Claudio Saavedra from comment #18)
> It's working for me in 3.18.0. That might be an issue with the web extension
> in the Fedora package. Please report to the Fedora developers.

...

(In reply to Michael Catanzaro from comment #19)
> (In reply to (bitlord) from comment #17)
> > This is nice feature and I like it, but looks like it disappeared on my
> > system, currently using epiphany-3.18.0-1.fc23.x86_64  it worked fine before
> > with some dev releases 3.17.x, problem is, I cannot remember when it stopped
> > working. Anyone else see this broken with final 3.18.x release(s)? (I can
> > open new bug report for this if needed)
> 
> Do you see any warnings when starting the browser in the terminal? Does
> password saving work? What about the Load Anyway button if you visit
> https://self-signed.badssl.com/ ?

I never save passwords, yes, invalid certificates are impossible to use, "Load Anyway" button doesn't work.

and 
"Error loading module '/usr/lib64/epiphany/3.18/web-extensions/libephywebextension.so': /usr/lib64/epiphany/3.18/web-extensions/libephywebextension.so: undefined symbol: gnome_desktop_thumbnail_factory_save_thumbnail"

so it is a downstream bug? Sorry I never looked at rh bugzilla for reports.

I think it's not a downstream bug, but a bug that will be arriving in other distros sooner or later. Let's use bug #755814.