[webkit-reviews] review denied: [Bug 23508] Returns Blank Page for all "about" protocols : [Attachment 26980] Patch and logfile

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 23 18:18:21 PST 2009


Darin Adler <darin at apple.com> has denied George Staikos <staikos at kde.org>'s
request for review:
Bug 23508: Returns Blank Page for all "about" protocols
https://bugs.webkit.org/show_bug.cgi?id=23508

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

------- Additional Comments from Darin Adler <darin at apple.com>
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.


More information about the webkit-reviews mailing list