Bug 24422 - REGRESSION: null-URL crash in FrameLoader setting location.hash on new window
Summary: REGRESSION: null-URL crash in FrameLoader setting location.hash on new window
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Darin Adler
URL: http://www.w3schools.com/js/tryit.asp...
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2009-03-06 06:39 PST by Darin Adler
Modified: 2009-03-09 11:29 PDT (History)
0 users

See Also:

Attachments
patch (15.69 KB, patch)
2009-03-06 07:52 PST, Darin Adler 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2009-03-06 06:39:11 PST 
The test at http://www.w3schools.com/js/tryit.asp?filename=tryjs_anchor1 triggers a crash inside the loader. I have a test case and fix underway.
Comment 1 Darin Adler 2009-03-06 07:52:31 PST 
Created attachment 28359 [details]
patch
Comment 2 Darin Fisher (:fishd, Google) 2009-03-06 08:23:01 PST 
Comment on attachment 28359 [details]
patch

>+shouldBe("testWindow.location.toString()", "'/'"); // Firefox returns about:blank
>+shouldBe("testWindow.location.href", "'/'"); // Firefox returns about:blank
>+shouldBe("testWindow.location.protocol", "':'"); // Firefox returns about:
>+shouldBe("testWindow.location.host", "''"); // Firefox throws an exception
>+shouldBe("testWindow.location.hostname", "''"); // Firefox throws an exception

This patch looks good to me.  (I verified that all of the URL strings have
been completed before the isEmpty checks.)  However, I wonder about all of
these behavior differences from Firefox.  Are those potential compat issues?
Comment 3 Darin Adler 2009-03-06 09:10:10 PST 
(In reply to comment #2)
> This patch looks good to me.  (I verified that all of the URL strings have
> been completed before the isEmpty checks.)  However, I wonder about all of
> these behavior differences from Firefox.  Are those potential compat issues?

They probably aren't but they might be. That's why I added those comments.

We should check the behavior of IE before making any changes. I'd definitely be open to changing this to be closer to the other browsers. And we should check if HTML 5 has anything to say about this.
Comment 4 Alexey Proskuryakov 2009-03-06 09:28:16 PST 
> They probably aren't but they might be. That's why I added those comments.

See bug 21597, currently in review queue.
Comment 5 Darin Adler 2009-03-06 09:30:38 PST 
Although I landed this patch, I just noticed that while the regression test works in Safari, in DumpRenderTree it's just FAIL, FAIL, FAIL, so I need to resolve that.
Comment 6 Darin Adler 2009-03-06 09:30:55 PST 
Comment on attachment 28359 [details]
patch

Clearing review flag since this was landed.