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 681506 - Show information about the SSL errors when clicking on lock icon
Show information about the SSL errors when clicking on lock icon
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-08-09 10:33 UTC by Carlos Garcia Campos
Modified: 2012-08-09 12:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Return the certificate and TLS errors in ephy_web_view_get_security_level (7.53 KB, patch)
2012-08-09 10:35 UTC, Carlos Garcia Campos
committed Details | Review
Patch (22.20 KB, patch)
2012-08-09 10:35 UTC, Carlos Garcia Campos
reviewed Details | Review
Updated patch (17.31 KB, patch)
2012-08-09 12:01 UTC, Carlos Garcia Campos
committed Details | Review

Description Carlos Garcia Campos 2012-08-09 10:33:30 UTC
We can show information about the specific errors, and the certificate details using libgcr if available.
Comment 1 Carlos Garcia Campos 2012-08-09 10:35:10 UTC
Created attachment 220771 [details] [review]
Return the certificate and TLS errors in ephy_web_view_get_security_level
Comment 2 Carlos Garcia Campos 2012-08-09 10:35:33 UTC
Created attachment 220772 [details] [review]
Patch
Comment 3 Xan Lopez 2012-08-09 10:56:09 UTC
Review of attachment 220771 [details] [review]:

Looks good to me.
Comment 4 Xan Lopez 2012-08-09 11:01:36 UTC
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.
Comment 5 Dan Winship 2012-08-09 11:27:43 UTC
(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.
Comment 6 Carlos Garcia Campos 2012-08-09 11:29:21 UTC
(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!
Comment 7 Carlos Garcia Campos 2012-08-09 12:01:03 UTC
Created attachment 220781 [details] [review]
Updated patch
Comment 8 Xan Lopez 2012-08-09 12:06:49 UTC
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 9 Carlos Garcia Campos 2012-08-09 12:55:12 UTC
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