23508 2009-01-23 12:47:02 -0800 Returns Blank Page for all "about" protocols 2009-02-02 13:50:00 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) All Linux RESOLVED FIXED P2 Normal --- 0 gen webkit-unassigned manyoso staikos zimmermann oldest_to_newest 106787 0 gen 2009-01-23 12:47:02 -0800 loader/MainResourceLoader.cpp: The function shouldLoadAsEmptyDocument() returns true for all "about" prefixed pages (eg "about:config") pages which blocks the implementation of the about protocol. Only "about:blank" should return true for a blank page. I've included a patch and a log file. 2009-01-23 Genevieve Mak <gen@staikos.net> Reviewed by NOBODY (OOPS!). Fix shouldLoadAsEmptyDocument() to return a blank page for an empty url or about:blamk * loader/MainResourceLoader.cpp: diff --git a/WebCore/loader/MainResourceLoader.cpp b/WebCore/loader/MainResourceLoader.cpp index 325809b..57c9426 100644 --- a/WebCore/loader/MainResourceLoader.cpp +++ b/WebCore/loader/MainResourceLoader.cpp @@ -186,7 +186,7 @@ void MainResourceLoader::willSendRequest(ResourceRequest& newRequest, const Reso static bool shouldLoadAsEmptyDocument(const KURL& url) { - return url.isEmpty() || equalIgnoringCase(String(url.protocol()), "about"); + return url.isEmpty() || (equalIgnoringCase(url.protocol(), "about") && equalIgnoringCase(url.path().stripWhiteSpace(), "blank")); } void MainResourceLoader::continueAfterContentPolicy(PolicyAction contentPolicy, const ResourceResponse& r) 106788 1 26980 gen 2009-01-23 12:51:10 -0800 Created attachment 26980 Patch and logfile Proposed patch to return a blank page for only "about:blank" urls and empty urls instead of all "about:*" urls. 106838 2 26980 darin 2009-01-23 18:18:21 -0800 Comment on attachment 26980 Patch and logfile Looks good. 1) The check for "about" should be using the protocolIs function, which is more efficient than calling .protocol() and then calling equalIgnoringCase. You're not changing that aspect of the function, but I'm surprised to see any code still doing it this old-fashioned way. 2) To limit this to "about:blank", then I think the normal idiom would be "equalIgnoringRef(url, blankURL())". This would be more efficient than the code you wrote. Your code would also allow strings like "about: blank " and "about:blank?notsoblank"; maybe that additional behavior is good, maybe bad. You 3) SecurityOrigin.cpp assumes that the "about" protocol has certain security properties. If you implement something that makes some about URLs return something other than an empty page, you could create security problems. I don't know anything more than that. 4) Safari and other Mac OS X applications may well rely on all "about:" URLs giving you a blank document, not just "about:blank", so I think this change may not be safe to make cross-platform. So I'm going to say review- until you either put this inside some kind of #if or provide some evidence that clients don't rely on this behavior on Mac OS X. 107092 3 27051 gen 2009-01-26 14:58:53 -0800 Created attachment 27051 Updated Patch Thanks, fixed issues #1 and #2. For issue #4 I've also put an "#if PLATFORM() around the fix in order to preserve the status quo. For #3 wouldn't it be up to whomever implements the browser to handle? I could see why you might want default blank pages for the "about" protocol but I'm not sure MainResourceLoader is the right place. When using Qt, MainResourceLoader gets a hold of the request before QNetworkReplyHandler even gets a chance to let the browser look at it. 108007 4 27051 staikos 2009-02-02 12:38:02 -0800 Comment on attachment 27051 Updated Patch Seems to have addressed the issues. Looks okay to me. 108028 5 staikos 2009-02-02 13:50:00 -0800 Applied in r40491 26980 2009-01-23 12:51:10 -0800 2009-01-26 14:58:53 -0800 Patch and logfile about.patch text/plain 1177 gen ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NzA4ODVlOS4uYTY4OWIyYyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMiBAQAorMjAwOS0wMS0yMyAgR2VuZXZpZXZlIE1h ayAgPGdlbkBzdGFpa29zLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBGaXggc2hvdWxkTG9hZEFzRW1wdHlEb2N1bWVudCgpIHRvIHJldHVybiBh IGJsYW5rIHBhZ2UgZm9yIGFuIGVtcHR5CisgICAgICAgIHVybCBvciBhYm91dDpibGFtaworCisg ICAgICAgICogbG9hZGVyL01haW5SZXNvdXJjZUxvYWRlci5jcHA6CisKIDIwMDktMDEtMTQgIEFk YW0gVHJlYXQgIDxhZGFtLnRyZWF0QHRvcmNobW9iaWxlLmNvbT4KIAogICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvbG9hZGVyL01haW5SZXNv dXJjZUxvYWRlci5jcHAgYi9XZWJDb3JlL2xvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCmlu ZGV4IDMyNTgwOWIuLjU3Yzk0MjYgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvbG9hZGVyL01haW5SZXNv dXJjZUxvYWRlci5jcHAKKysrIGIvV2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNw cApAQCAtMTg2LDcgKzE4Niw3IEBAIHZvaWQgTWFpblJlc291cmNlTG9hZGVyOjp3aWxsU2VuZFJl cXVlc3QoUmVzb3VyY2VSZXF1ZXN0JiBuZXdSZXF1ZXN0LCBjb25zdCBSZXNvCiAKIHN0YXRpYyBi b29sIHNob3VsZExvYWRBc0VtcHR5RG9jdW1lbnQoY29uc3QgS1VSTCYgdXJsKQogewotICAgIHJl dHVybiB1cmwuaXNFbXB0eSgpIHx8IGVxdWFsSWdub3JpbmdDYXNlKFN0cmluZyh1cmwucHJvdG9j b2woKSksICJhYm91dCIpOworICAgIHJldHVybiB1cmwuaXNFbXB0eSgpIHx8IChlcXVhbElnbm9y aW5nQ2FzZSh1cmwucHJvdG9jb2woKSwgImFib3V0IikgJiYgZXF1YWxJZ25vcmluZ0Nhc2UodXJs LnBhdGgoKS5zdHJpcFdoaXRlU3BhY2UoKSwgImJsYW5rIikpOwogfQogCiB2b2lkIE1haW5SZXNv dXJjZUxvYWRlcjo6Y29udGludWVBZnRlckNvbnRlbnRQb2xpY3koUG9saWN5QWN0aW9uIGNvbnRl bnRQb2xpY3ksIGNvbnN0IFJlc291cmNlUmVzcG9uc2UmIHIpCg== 27051 2009-01-26 14:58:53 -0800 2009-02-02 12:38:02 -0800 Updated Patch about2.patch text/plain 1270 gen ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NzA4ODVlOS4uYTY4OWIyYyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMiBAQAorMjAwOS0wMS0yMyAgR2VuZXZpZXZlIE1h ayAgPGdlbkBzdGFpa29zLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBGaXggc2hvdWxkTG9hZEFzRW1wdHlEb2N1bWVudCgpIHRvIHJldHVybiBh IGJsYW5rIHBhZ2UgZm9yIGFuIGVtcHR5CisgICAgICAgIHVybCBvciBhYm91dDpibGFtaworCisg ICAgICAgICogbG9hZGVyL01haW5SZXNvdXJjZUxvYWRlci5jcHA6CisKIDIwMDktMDEtMTQgIEFk YW0gVHJlYXQgIDxhZGFtLnRyZWF0QHRvcmNobW9iaWxlLmNvbT4KIAogICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvbG9hZGVyL01haW5SZXNv dXJjZUxvYWRlci5jcHAgYi9XZWJDb3JlL2xvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCmlu ZGV4IDMyNTgwOWIuLjc1NDgzY2YgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvbG9hZGVyL01haW5SZXNv dXJjZUxvYWRlci5jcHAKKysrIGIvV2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNw cApAQCAtMTg2LDcgKzE4NiwxMSBAQCB2b2lkIE1haW5SZXNvdXJjZUxvYWRlcjo6d2lsbFNlbmRS ZXF1ZXN0KFJlc291cmNlUmVxdWVzdCYgbmV3UmVxdWVzdCwgY29uc3QgUmVzbwogCiBzdGF0aWMg Ym9vbCBzaG91bGRMb2FkQXNFbXB0eURvY3VtZW50KGNvbnN0IEtVUkwmIHVybCkKIHsKLSAgICBy ZXR1cm4gdXJsLmlzRW1wdHkoKSB8fCBlcXVhbElnbm9yaW5nQ2FzZShTdHJpbmcodXJsLnByb3Rv Y29sKCkpLCAiYWJvdXQiKTsKKyNpZiBQTEFURk9STShUT1JDSE1PQklMRSkKKyAgICByZXR1cm4g dXJsLmlzRW1wdHkoKSB8fCAodXJsLnByb3RvY29sSXMoImFib3V0IikgJiYgZXF1YWxJZ25vcmlu Z1JlZih1cmwsIGJsYW5rVVJMKCkpKTsKKyNlbHNlIAorICAgIHJldHVybiB1cmwuaXNFbXB0eSgp IHx8IHVybC5wcm90b2NvbElzKCJhYm91dCIpOworI2VuZGlmCiB9CiAKIHZvaWQgTWFpblJlc291 cmNlTG9hZGVyOjpjb250aW51ZUFmdGVyQ29udGVudFBvbGljeShQb2xpY3lBY3Rpb24gY29udGVu dFBvbGljeSwgY29uc3QgUmVzb3VyY2VSZXNwb25zZSYgcikKZGlmZiAtLWdpdCBhL1dlYktpdC5w cm8gYi9XZWJLaXQucHJvCg==