Created attachment 282841 [details] [review]
ephy-uri-helpers: Small fixes in tracking removal code

Created attachment 282842 [details] [review]
ephy-uri-helpers: Add helper to add GNOME's Amazon referral codes

So that we automatically give money to GNOME! (but only when setup to do
so, obviously).

Created attachment 282843 [details] [review]
embed: Add Amazon tags when set up to do so

This is untested, as my WebKitGTK is too old, but dropping a file like this one, should automatically load the tags when they're absent:
$ cat ~/.config/epiphany/amazon-referral.ini
[Amazon Referral Codes]
www.amazon.co.uk=hadessnet-21
www.amazon.de=hadessnet0f-21
www.amazon.es=hadessnet08-21
www.amazon.fr=hadessnet0b-21
www.amazon.it=hadessnet0e-21

(that's my own referral code, we would obviously have a file that's GNOME's referral code if we ever deploy this feature).

Review of attachment 282842 [details] [review]:

Worth mentioning that your sample key file is missing www.amazon.com

::: lib/ephy-uri-helpers.c
@@ +297,3 @@
+    return ret;
+
+  host = soup_uri_get_host (uri);

This could be NULL e.g. if it's an about: URI; you should bail if so.

@@ +302,3 @@
+    goto bail;
+
+  query = soup_uri_get_query (uri);

If I type www.amazon.co.uk then the query will always be null; this is the ?key=value&key=value portion of the URL. So you'll always bail here before you get a chance to append your referrer.

Created attachment 282878 [details] [review]
ephy-uri-helpers: Add helper to add GNOME's Amazon referral codes

So that we automatically give money to GNOME! (but only when setup to do
so, obviously).

Review of attachment 282843 [details] [review]:

::: embed/web-extension/ephy-web-extension.c
@@ +149,3 @@
+  if (new_uri) {
+    webkit_uri_request_set_uri (request, new_uri);
+    request_uri = webkit_uri_request_get_uri (request);

Now ephy_add_amazon_referral() works, but this variable is later used only to test whether the page should be blocked by adblock.

(In reply to comment #7)
> Review of attachment 282843 [details] [review]:
> 
> ::: embed/web-extension/ephy-web-extension.c
> @@ +149,3 @@
> +  if (new_uri) {
> +    webkit_uri_request_set_uri (request, new_uri);
> +    request_uri = webkit_uri_request_get_uri (request);
> 
> Now ephy_add_amazon_referral() works, but this variable is later used only to
> test whether the page should be blocked by adblock.

Is that a problem, or do we automatically assume that anything coming from amazon isn't adverts? It could be...

The problem is the new URI with the referrer appended is not used for anything else. (I still wind up at www.amazon.co.uk without the referrer.)

(In reply to comment #9)
> The problem is the new URI with the referrer appended is not used for anything
> else. (I still wind up at www.amazon.co.uk without the referrer.)

No, we call webkit_uri_request_set_uri() with that new URI. It's the same code as when we remove analytics just above (unless that's not working either...).

(In reply to comment #10)
> No, we call webkit_uri_request_set_uri() with that new URI. It's the same code
> as when we remove analytics just above (unless that's not working either...).

Doesn't seem to be working, e.g. http://www.imdb.com/news/ni57577848/?ref_=hm_nw_tp_t4 should be stripped but isn't.

But I'm not sure what's wrong. You're right that changing the URIRequest passed to the send-request signal handler ought to change the page that's loaded.

(In reply to comment #11)
> But I'm not sure what's wrong. You're right that changing the URIRequest passed
> to the send-request signal handler ought to change the page that's loaded.

Checking with Wireshark, it's working properly, I think, it's just that the change does not appear in Epiphany's address bar.

I think WebKitWebPage::willSendRequestForFrame() needs to update the WebView's address property if the URIRequest is different after the send-request signal is emitted. Carlos, is that correct? Is there any reasonable way to accomplish that?

Comment on attachment 282841 [details] [review]
ephy-uri-helpers: Small fixes in tracking removal code

Attachment 282841 [details] pushed as d5e3f0b - ephy-uri-helpers: Small fixes in tracking removal code

Comment on attachment 282843 [details] [review]
embed: Add Amazon tags when set up to do so

Removing from the unreviewed patches list, since there is an outstanding problem ("I think WebKitWebPage::willSendRequestForFrame() needs to update the WebView's address property if the URIRequest is different after the send-request signal is emitted.")

(In reply to Michael Catanzaro from comment #15)
> Comment on attachment 282843 [details] [review] [review]
> embed: Add Amazon tags when set up to do so
> 
> Removing from the unreviewed patches list, since there is an outstanding
> problem ("I think WebKitWebPage::willSendRequestForFrame() needs to update
> the WebView's address property if the URIRequest is different after the
> send-request signal is emitted.")

Is that a bug that needs to be filed against WebKit?

Oh, yes; I didn't read my own comment closely enough. This should be easy.

(In reply to Michael Catanzaro from comment #17)
> This should be easy.

Ha, ha, ha.....

Review of attachment 282841 [details] [review]:

Hm, this one is trivial

Review of attachment 282843 [details] [review]:

In the commit message, /s/embed/web-extension/

Changes are good

Review of attachment 282878 [details] [review]:

Thoughts on this:

* My primary concern: I want to see approval from our pro bono counsel, since this could possibly be an... issue.
* If approved, we should install the keyfile by default. (I presume you were planning to do this; otherwise, I don't see the advantage of setting it up manually over using Amazon Smile.)
* Ideally I would finish WebKit 146306 first.
* Code changes are good.

::: lib/ephy-uri-helpers.c
@@ +264,3 @@
+    g_key_file_unref (keyfile);
+    g_free (path);
+    return NULL;

The 'goto out' style would work better for this cleanup.

@@ +306,3 @@
+
+  query = soup_uri_get_query (uri);
+  if (query) {

You could probably split this into another function, gboolean ephy_should_add_amazon_referral().

@@ +338,3 @@
+bail:
+  g_clear_pointer (&tag, g_free);
+  g_list_free_full (items, (GDestroyNotify) query_item_free);

This cast is necessary?

Comment on attachment 282841 [details] [review]
ephy-uri-helpers: Small fixes in tracking removal code

Was committed in d5e3f0b6dceaf22d03cc13ef633a93317100be6b

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/epiphany/issues/247.