Bug 24422

Summary: REGRESSION: null-URL crash in FrameLoader setting location.hash on new window
Product: WebKit Reporter: Darin Adler <darin>
Component: Page LoadingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal Keywords: HasReduction, InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://www.w3schools.com/js/tryit.asp?filename=tryjs_anchor1
Attachments:
Description Flags
patch none

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.