Created attachment 220771 [details] [review]
Return the certificate and TLS errors in ephy_web_view_get_security_level

Created attachment 220772 [details] [review]
Patch

Review of attachment 220771 [details] [review]:

Looks good to me.

Review of attachment 220772 [details] [review]:

::: configure.ac
@@ +230,3 @@
+if test "$have_gcr" = "yes"; then
+        AC_DEFINE([HAVE_GCR], [1], [Define to compile with gcr support])
+fi

Why would we want to make this optional exactly? I think we should just make it a dependency.

::: lib/widgets/ephy-certificate-dialog.c
@@ +1,1 @@
+/*

The whole file seems to follow the old coding style apparently. It shouldn't.

(In reply to comment #4)
> Why would we want to make this optional exactly? I think we should just make it
> a dependency.

Yeah, I think a bunch of things are going to start depending on gcr soon.

(In reply to comment #4)
> Review of attachment 220772 [details] [review]:
> 
> ::: configure.ac
> @@ +230,3 @@
> +if test "$have_gcr" = "yes"; then
> +        AC_DEFINE([HAVE_GCR], [1], [Define to compile with gcr support])
> +fi
> 
> Why would we want to make this optional exactly? I think we should just make it
> a dependency.

Well, it's not a stable API (you have to define GCR_API_SUBJECT_TO_CHANGE to use it), and it's not something required to make ephy work. But still, if you want to make it a hard dependency, I'll rework the patch.

> ::: lib/widgets/ephy-certificate-dialog.c
> @@ +1,1 @@
> +/*
> 
> The whole file seems to follow the old coding style apparently. It shouldn't.

I didn't know there was new and old coding styles, I just followed coding styles of other files in the same directory. So, I guess the new one is what it's currently documented in HACKING, right? I'll rewrite the patch.

Thanks for reviewing!

Created attachment 220781 [details] [review]
Updated patch

Review of attachment 220781 [details] [review]:

Looks fine, you can just commit fixing the style thing and adding a min version for gcr if you feel like it.

::: configure.ac
@@ +131,3 @@
 		  libnotify >= $LIBNOTIFY_REQUIRED
 		  sqlite3
+                  gcr-3

We don't want to set a minimum version? Especially if the API is changing it seems like a good idea.

::: lib/widgets/ephy-certificate-dialog.c
@@ +109,3 @@
+  if (tls_errors & G_TLS_CERTIFICATE_NOT_ACTIVATED)
+    g_ptr_array_add (errors, _("The certificate activation time is still in the future"));
+

Pretty much all the braces starting from here are wrong. Sorry to be a pain in the ass!

Comment on attachment 220781 [details] [review]
Updated patch

Pushed, hope coding style is correct now :-P I also updated jhbuild to make ephy require gcr