WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
23508
Returns Blank Page for all "about" protocols
https://bugs.webkit.org/show_bug.cgi?id=23508
Summary
Returns Blank Page for all "about" protocols
Genevieve Mak
Reported
2009-01-23 12:47:02 PST
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)
Attachments
Patch and logfile
(1.15 KB, patch)
2009-01-23 12:51 PST
,
Genevieve Mak
darin
: review-
Details
Formatted Diff
Diff
Updated Patch
(1.24 KB, patch)
2009-01-26 14:58 PST
,
Genevieve Mak
staikos
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Genevieve Mak
Comment 1
2009-01-23 12:51:10 PST
Created
attachment 26980
[details]
Patch and logfile Proposed patch to return a blank page for only "about:blank" urls and empty urls instead of all "about:*" urls.
Darin Adler
Comment 2
2009-01-23 18:18:21 PST
Comment on
attachment 26980
[details]
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.
Genevieve Mak
Comment 3
2009-01-26 14:58:53 PST
Created
attachment 27051
[details]
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.
George Staikos
Comment 4
2009-02-02 12:38:02 PST
Comment on
attachment 27051
[details]
Updated Patch Seems to have addressed the issues. Looks okay to me.
George Staikos
Comment 5
2009-02-02 13:50:00 PST
Applied in
r40491
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug