[webkit-reviews] review granted: [Bug 53850] Make WebKit's fragment cannonicalization match other browsers : [Attachment 81357] 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 53850: Make WebKit's fragment cannonicalization match other browsers
https://bugs.webkit.org/show_bug.cgi?id=53850

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

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

Some nits below.  w.r.t. diverging from Firefox.  I discussed this topic (in
general) with some Firefox folks.  They said they're willing to converge to
match other browsers URL handling as long as we write a spec for how we think
things ought to be.  If we follow this trajectory, everyone should end up with
interoperable behavior.

> Source/WebCore/platform/KURL.cpp:997
> +	   if (c == '\0')
> +	       continue;

This part isn't correct.  It's an artifact of how Brett did his testing in IE
(which strips nulls at the HTML parser layer).	It's hard to observe directly,
but we should be able to observe via the DOM (not via the HTML parser).

> Source/WebCore/platform/KURL.cpp:1001
> +	       *p++ = '%';
> +	       *p++ = hexDigits[c >> 4];
> +	       *p++ = hexDigits[c & 0xF];

We should add an inline helper function to do this work instead of copy/pasting
it.  Really what we should do is a templated free function that takes a
predicate for what to escape, if you want to get fancy.