[webkit-reviews] review denied: [Bug 61097] Implement HTML5 location resolveURL : [Attachment 94160] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 19 21:28:44 PDT 2011


Adam Barth <abarth at webkit.org> has denied Erik Arvidsson <arv at chromium.org>'s
request for review:
Bug 61097: Implement HTML5 location resolveURL
https://bugs.webkit.org/show_bug.cgi?id=61097

Attachment 94160: Patch
https://bugs.webkit.org/attachment.cgi?id=94160&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94160&action=review

r- for compile break.  One comment below:

> Source/WebCore/platform/KURL.h:154
> +    String prettyURLWithTrailingSlash() const;

You probably need this in KURLGoogleWhateverThatForkedFileIsCalled.cpp as well.
 Is prettyURL called elsewhere without adding a trailing slash? 
prettyURLWithTrailingSlash isn't the greatest name because there isn't always a
trailing slash, as in:

http://example.com/foo?bar

Also, does this code handle the following input correctly:

http://example.com?bar

I guess it depends on KURL canonicalizes things.  I don't think hasPath() will
ever be false for GURL because GURL is more agressive about canonicalizing. 
Another interesting test case is:

http:

but now we're getting into UA-specific URL behaviors.


More information about the webkit-reviews mailing list