Created attachment 236724 [details] [review]
Patch

Review of attachment 236724 [details] [review]:

Looks pretty good to me, just two small nitpicks!

::: embed/web-extension/ephy-web-extension.c
@@ +286,3 @@
+ephy_web_extension_get_web_app_title (WebKitWebPage *web_page)
+{
+  WebKitDOMDocument *document = NULL;

This is super nitpick, but no need to set this to NULL here.

@@ +363,3 @@
+    g_dbus_method_invocation_return_value (invocation,
+                                           g_variant_new ("(s)",
+                                                          title != NULL ? title : ""));

Indentation seems off here, and you can just use title ? title : ""

Created attachment 237339 [details] [review]
Patch fixing small issues commented before

Fixed the small issues commented by Xan.

Please push it for me as I don't have permissions at git.gnome.org. Thanks :-)

Created attachment 237343 [details] [review]
Patch including check for null web extension

After bug #694519 web extension proxy can be NULL, so included a check before using it to get the title from DOM.

Created attachment 237354 [details] [review]
Patch 1/2

Created attachment 237355 [details] [review]
Patch 2/2

This has been implemented now in 2 patches:
1) It moves the code of the DOM bindings to ephy-web-dom-utils
2) Implements get title in WK2

If you prefer I can merge it in just one patch.

Created attachment 237456 [details] [review]
Patch 2/2

Review of attachment 237354 [details] [review]:

::: lib/ephy-web-dom-utils.c
@@ +134,3 @@
+  }
+
+  return title;

The opening braces are wrong here, we should be using the style described in HACKING.

Review of attachment 237456 [details] [review]:

::: src/window-commands.c
@@ +725,2 @@
 	{
+		g_print ("inside last if");

Oops ;)

@@ +770,3 @@
+				   NULL,
+				   fill_default_application_title_callback,
+				   data);

Having GDbus calls around the place is pretty ugly, I think in the future we should just have wrappers around them in EphyWebView or wherever. But should do for now.

Created attachment 237551 [details] [review]
Patch 1/1 updated

Created attachment 237552 [details] [review]
Patch 1/2 updated

(In reply to comment #9)
> Review of attachment 237456 [details] [review]:

> Having GDbus calls around the place is pretty ugly, I think in the future we
> should just have wrappers around them in EphyWebView or wherever. But should do
> for now.

I agree, we should have some methods, perhaps in EphyWebExtension to wrap the dbus calls.

Review of attachment 237551 [details] [review]:

::: lib/ephy-web-dom-utils.c
@@ +108,3 @@
+ **/
+char *
+ephy_web_dom_get_application_title (WebKitDOMDocument *document)

Sorry to be a pain in the ass, but the name is also wrong here.

Review of attachment 237552 [details] [review]:

Looks good minus the naming issue.

(In reply to comment #13)
> Review of attachment 237551 [details] [review]:
> 
> ::: lib/ephy-web-dom-utils.c
> @@ +108,3 @@
> + **/
> +char *
> +ephy_web_dom_get_application_title (WebKitDOMDocument *document)
> 
> Sorry to be a pain in the ass, but the name is also wrong here.

Don't worry, it's my fault. I hope to do it better next time.

I'll upload the patches updated.

Created attachment 237658 [details] [review]
Patch 1/2 fixed naming issue

Created attachment 237659 [details] [review]
Patch 2/2 fixed naming issue

Review of attachment 237658 [details] [review]:

Looks good.

Review of attachment 237659 [details] [review]:

Looks good.

Pushed both patches to master.