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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 12 22:03:40 PDT 2010


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

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
2     // First we try a full-match, for cases such as 'youtube.com' as opposed
to
 193	 // 'www.youtube.com'.
 194	 if (googleDomains.contains(host))
 195	     return true;

That code will never be hit with the current list of domains.

I would not have architected this check this way.

google.* is the only one which want's a "namespace" an only so you can look for
the string ".google." and then look up the final tail in a list.  All the other
domains only need .com and could have used domain postfixes instead of needing
the namespace architecture.

Repeating the string "google." in all of your googleDomains seems unecessary.

The above match against the shortened domain seems wrong in all of the domain
examples given.


More information about the webkit-reviews mailing list