[webkit-reviews] review denied: [Bug 30225] window.location.href needlessly decodes URI-encoded characters in the URI path : [Attachment 40896] Make KURL::prettyUrl (used by window.location.href) not URI-decode the URI path.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 8 12:11:50 PDT 2009


Eric Seidel <eric at webkit.org> has denied Andrei Popescu <andreip at google.com>'s
request for review:
Bug 30225: window.location.href needlessly decodes URI-encoded characters in
the URI path
https://bugs.webkit.org/show_bug.cgi?id=30225

Attachment 40896: Make KURL::prettyUrl (used by window.location.href) not
URI-decode the URI path.
https://bugs.webkit.org/attachment.cgi?id=40896&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
You ChangeLog is duplicated.

I don't really like pathRaw(), maybe encodedPath()?  Or should path be
decodedPath()?

fast/dom/Window/Location/location-path-has-is-unmodified.htm is sorta a js
test, except you didn't generate the tempate using make-script-test-wrappers.

Why does this need to be run from onload?  It needs to wait for the iframe to
load?  Will body onload do that?

(If this needs to wait for onload, then it won't work quite right as a js test,
so maybe that's why you ddid it this way.)

+	 correctValue = "path%2Dwith-escape%2Dcharacters.html";
+	 shouldBe("result", "correctValue");

shouldBeEqualToString("normalizeURL(String(window.frames[0].location.href))",
"path%2Dwith-escape%2Dcharacters.html");
would have been better.

Does this actually need to load a valid resource to work?


More information about the webkit-reviews mailing list