[webkit-reviews] review denied: [Bug 29972] Allow JavaScript to manipulate parts of the URL in href attribute. : [Attachment 40460] Patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 1 11:26:50 PDT 2009


Darin Adler <darin at apple.com> has denied Yael <yael.aharon at nokia.com>'s request
for review:
Bug 29972: Allow JavaScript to manipulate parts of the URL in href attribute.
https://bugs.webkit.org/show_bug.cgi?id=29972

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

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good!

The regression test should be dumpAsText(). And ideally it should be one of the
script-tests. This seems like a perfect candidate for that style of test.

This is a test of HTMLAnchorElement, so it should be in
fast/dom/HTMLAnchorElement, not fast/dom/Element. So you should create that new
directory.

We need a lot more test cases, with all the edge cases like passing in
malformed strings, like including a ":" or other punctuation in the protocol
string and many other examples like that. And we need to see how the abnormal
cases behave in other browsers to see if our behavior is right. There's no
reason to assume the KURL functions just happen to do everything right already.


For changes like this one of the most important issues is how similar or
different our behavior is from other browsers.

review- because the test needs changes, at the very least to dumpAsText(), but
I'd prefer if you did everything I mention above.


More information about the webkit-reviews mailing list