[webkit-reviews] review requested: [Bug 39617] [GTK] Google sites do not like WebKitGTK+ : [Attachment 64328] proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 13 05:56:54 PDT 2010


Gustavo Noronha (kov) <gns at gnome.org> has asked  for review:
Bug 39617: [GTK] Google sites do not like WebKitGTK+
https://bugs.webkit.org/show_bug.cgi?id=39617

Attachment 64328: proposed patch
https://bugs.webkit.org/attachment.cgi?id=64328&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
OK, I think I now understand what you're saying - you mean the postfixes are
not needed for anything that is not .google., sorry for being confused! I
reworked the patch to only do the postfix checking for .google., and I do the
postfix-only checks for the others, as you suggest. Let me also comment on your
other messages:

> My r- is perhaps a bit strong, but this code still seems confused.

It's fine, I just failed to understand what you meant.

> WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:171
>  +	  googleDomains.add("gmail.com");
> This always redirects to mail.google.com.  No need for explicit inclusion
here.

I would prefer to have checks even for the redirect-only domains in case they
are repurposed in any way. It's not required, sure, but it's not bad to have it
either, so I would prefer keeping it.

> My only current concern with this patch is that you're casting your net a bit
too wide.  Generally with site-specific spoofing the goal is to only spoof for
the absolute minimum number of pages, and make it possible for there to be a
path away from the spoofing.  I think that Gtk needs this patch (and will
probably need it for a long time), but the end goal should be to have a world
where this patch is not needed.

I understand your concern, but there are a few things I would like to point out
about why I planned this net to be cast widely:

First off, we don't know to which extent Google is excluding us from content
based on User-Agent sniffing - the fact that I only learned about the Google
images improvement after writing the patch is an example of that - not being a
hardcore google user myself, it's hard for me to even try to make a list of
issues. Also, there are distributions which are going to be shipping browsers
with WebKitGTK+ which will not have them updated to newer versions for months,
years perhaps. Even though I consider that a shortcoming of the usual
distribution process of FOSS distributions, it's something we have to account
for when trying to decide what to do. Doing it this way we avoid having random
Google services start failing to work for those people because they started
doing broken sniffing (as was the case with Youtube).

The solution is far from perfect, I agree, and I would love to not ever need
this patch - I have even been trying to fix the sniffing that Google does
instead since around January this year because I really hate to have this kind
of thing in our code, but with the results we got up to now, I think for now
it's a good first step in assuring we won't have people saying Epiphany doesn't
work when the next cool pacman-style doodle comes out.


More information about the webkit-reviews mailing list