[webkit-reviews] review denied: [Bug 21597] Set popup's location to about:blank while it's loading : [Attachment 28203] Location.cpp patch with new expected results.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Mike Belshe <mike at belshe.com>'s
request for review:
Bug 21597: Set popup's location to about:blank while it's loading
https://bugs.webkit.org/show_bug.cgi?id=21597

Attachment 28203: Location.cpp patch with new expected results.
https://bugs.webkit.org/attachment.cgi?id=28203&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
This looks like a good approach. It would also be possible to change the
FrameLoader itself to use the blankURL instead of an empty URL -- riskier and
harder to get it right.

You'll also have to patch the test and results from the test I just added, by
modifying fast/dom/resources/location-new-window-no-crash.js and updating
fast/dom/location-new-window-no-crash-expected.txt with new results.

The ChangeLog mentions a function WebCore::urlWhileLoading -- this version of
the patch doesn't have that so it shouldn't be in the change log.

Those change logs are no good -- they don't say anything about what you're
changing and why, nor do they have the URL of this bug! The change log needs to
mention the rationale about why these values need to be returned. I'd like some
more data about the behavior of IE here and how you determined this behavior
was required.