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 739787 - Port to WebKit2
Port to WebKit2
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
3.15.x
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
Depends on:
Blocks: 686373
 
 
Reported: 2014-11-07 15:06 UTC by Debarshi Ray
Modified: 2014-11-11 17:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port to WebKit2 (4.91 KB, patch)
2014-11-07 15:12 UTC, Debarshi Ray
reviewed Details | Review
Port to WebKit2 (4.88 KB, patch)
2014-11-10 13:51 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2014-11-07 15:06:52 UTC
WebKit2 is the way forward and newer WebKitGTK+ releases don't have the old WK1 API any more.
Comment 1 Debarshi Ray 2014-11-07 15:12:51 UTC
Created attachment 290175 [details] [review]
Port to WebKit2
Comment 2 Cosimo Cecchi 2014-11-07 19:47:25 UTC
Review of attachment 290175 [details] [review]:

Looks good to me, other than the comment below.

::: src/edit.js
@@ +53,3 @@
+        context = WebKit.WebContext.get_default();
+        cookie_manager = context.get_cookie_manager();
+        cookie_manager.delete_all_cookies();

Why do you need this?
Comment 3 André Klapper 2014-11-09 01:05:54 UTC
Feel free to use http://landfill.bugzilla.org/ if you want to test things.
Comment 4 André Klapper 2014-11-09 01:06:44 UTC
Whoops, damn, wrong tab. Sorry.
Comment 5 Debarshi Ray 2014-11-10 13:50:30 UTC
(In reply to comment #2)
> Review of attachment 290175 [details] [review]:
> 
> Looks good to me, other than the comment below.
> 
> ::: src/edit.js
> @@ +53,3 @@
> +        context = WebKit.WebContext.get_default();
> +        cookie_manager = context.get_cookie_manager();
> +        cookie_manager.delete_all_cookies();
> 
> Why do you need this?

In the past we had code to remove the existing cookie jar with our own to ensure that the cookie jar used in the browser does not leak into our session. This idea originated in GOA [1] where we wanted to be very sure that the user explicitly entered her credentials, instead of accidentally adding somebody else's account that might have been used in the browser.

Anyway, as far as I can see cookies are not leaking from the browser to the application, so we don't need this delete_all_cookies call. I tested by logging into Drive in epiphany and then starting gnome-documents without the cookies DB.

Note that if you ctrl+c out of the app instead of waiting for the inactivity-timeout, then the cookies won't be stored persistently and you will have to re-authenticate next time.

[1] https://git.gnome.org/browse/gnome-online-accounts/tree/src/goabackend/goawebview.c#n286
Comment 6 Debarshi Ray 2014-11-10 13:51:28 UTC
Created attachment 290342 [details] [review]
Port to WebKit2

Remove the delete_all_cookies call.
Comment 7 Cosimo Cecchi 2014-11-10 17:43:22 UTC
Review of attachment 290342 [details] [review]:

Looks good, thanks.
Comment 8 Carlos Garcia Campos 2014-11-11 17:07:43 UTC
(In reply to comment #5)
> (In reply to comment #2)
> > Review of attachment 290175 [details] [review] [details]:
> > 
> > Looks good to me, other than the comment below.
> > 
> > ::: src/edit.js
> > @@ +53,3 @@
> > +        context = WebKit.WebContext.get_default();
> > +        cookie_manager = context.get_cookie_manager();
> > +        cookie_manager.delete_all_cookies();
> > 
> > Why do you need this?
> 
> In the past we had code to remove the existing cookie jar with our own to
> ensure that the cookie jar used in the browser does not leak into our session.

That shouldn't happen unless you use the same sqlite database, which is not the case.

> This idea originated in GOA [1] where we wanted to be very sure that the user
> explicitly entered her credentials, instead of accidentally adding somebody
> else's account that might have been used in the browser.
> 
> Anyway, as far as I can see cookies are not leaking from the browser to the
> application, so we don't need this delete_all_cookies call. I tested by logging
> into Drive in epiphany and then starting gnome-documents without the cookies
> DB.

In WebKit2 the application has to set a cookies jar path to enable persisten cookies, so unless two apps use the same file the cookies should never be shared between applications.
 
> Note that if you ctrl+c out of the app instead of waiting for the
> inactivity-timeout, then the cookies won't be stored persistently and you will
> have to re-authenticate next time.

We write the cookies to the sqlite database when the cookie jar changes, so that sounds like a bug in wk, please file a bug report.

> [1]
> https://git.gnome.org/browse/gnome-online-accounts/tree/src/goabackend/goawebview.c#n286

I'm happy to see the port was so easy for gnome-documents :-)