[webkit-reviews] review granted: [Bug 54090] KURL should remove default port numbers when cannonicalizing urls (to match every other browser) : [Attachment 81790] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 54090: KURL should remove default port numbers when cannonicalizing urls
(to match every other browser)
https://bugs.webkit.org/show_bug.cgi?id=54090

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

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

> Source/WebCore/platform/KURL.cpp:1105
> +static inline bool isDefaultPortForScheme(const char* port, size_t
portLength, const char* scheme, size_t schemeLength)

Where did this list of schemes/ports come from?  I think Firefox has some sort
of extensibility model where each URL provider annonces a scheme and a default
port.  This is probably a good step though.

> Source/WebCore/platform/KURL.cpp:1122
> +    switch (schemeLength) {
> +    case 4:
> +	   return !strncmp(scheme, "http", schemeLength) && !strncmp(port,
"80", portLength);
> +    case 5:
> +	   return !strncmp(scheme, "https", schemeLength) && !strncmp(port,
"443", portLength);
> +    case 3:
> +	   if (!strncmp(scheme, "ftp", schemeLength))
> +	       return !strncmp(port, "43", portLength);
> +	   if (!strncmp(scheme, "wss", schemeLength))
> +	       return !strncmp(port, "443", portLength);
> +	   break;
> +    case 6:
> +	   return !strncmp(scheme, "gopher", schemeLength) && !strncmp(port,
"70", portLength);
> +    case 2:
> +	   return !strncmp(scheme, "ws", schemeLength) && !strncmp(port, "80",
portLength);
> +    }

Can you put these in numerical origin?	Also, consider adding an explicit
default case.