To reproduce: 1. Open a new tab. 2. Type javascript:void(location.href='http://www.google.com') 3. Press return. WebKit should almost immediately exit.
Created attachment 6525 [details] Crash Report
Confirmed. Crashers and regressions are P1. Added appropriate keywords. Works fine on Safari 2.0.3 (417.8) on 10.4.5. Crashes on locally-built ToT WebKit r12813 (anonsvn).
Severity -> critical for crashers.
I think I probably caused this w/ recent refactoring work.
This is indeed a regression from r12811 (Improvements to frame loading). The crash happens because the frame exists but document() is nil here: void Location::put(ExecState *exec, const Identifier &p, JSValue *v, int attr) [...] Frame* p = Window::retrieveActive(exec)->frame(); if ( p ) url = p->document()->completeURL( str ); According to Maciej on IRC, a document used to be created as necessary before reaching here. The ultimate reason why this doesn't happen after r12811 is that previously -[WebCoreFrameBridge openURL:reload:contentType:refresh:lastModified:pageCache:] was called with 'about:blank' in this case, and now it is called with an empty NSURL. (The call site used to be -[WebDataSource(WebPrivate) _commitIfReady:] and now it's coming from -[WebFrame(WebPrivate) _commitProvisionalLoad:]). There seems to be another code path that creates a document on-demand, since entering javascript:document in the address bar of a newly-opened window works (and instantiates an HTMLDocumentImpl), and I think that's the code path that should kick in in the case of this bug as well. Having said that, another side-effect of the current state of things is that if you enter javascript: URLs in the address bar of a newly opened window, it will now erase the address bar after each URL, whereas previously it only did so after the first one (when there was no document). I believe that there is already a radar on the annoying first-time behavior.
*** Bug 7403 has been marked as a duplicate of this bug. ***
Created attachment 6729 [details] Patch This fixes the crash, but I'm pretty sure it's not the right fix.
*** Bug 7428 has been marked as a duplicate of this bug. ***
Bug 7428 has a reduction that could easily turn into a layout test, although we should write another test for the javascript: case if we can.
Comment on attachment 6729 [details] Patch I'm going to post a version with a layout test based on bug 7428 and a change log entry.
Created attachment 6743 [details] Patch, including test and changelog
Comment on attachment 6743 [details] Patch, including test and changelog Looks fine. r=me
(In reply to comment #7) > This fixes the crash, but I'm pretty sure it's not the right fix. I'd love to find a cleaner fix for this, but it's best to get this fix in and the layout test that checks the problematic case. We can always land a different fix later.
Landed.