On empty fragment url's, document::gotoanchor crashes browser. for ex: <A href="#"> link </a> Default behavior of empty fragment is to first check if cssTarget is specified, if not goto first element. In case if this cssTarget node is delete by Javascript, browser crashes clicking on "link" in the example.
Created attachment 25670 [details] fix for empty fragment URL crash assigning m_cssTarget to null if that node is getting deleted.
Could you provide a test case as well?
*** Bug 22591 has been marked as a duplicate of this bug. ***
Created attachment 25712 [details] test case test case. click on first link, then click second and finally on third link!
Comment on attachment 25670 [details] fix for empty fragment URL crash The fix looks fine. Now we need a regression test for this.
Please let me know what is the next thing I can do ?
(In reply to comment #6) > Please let me know what is the next thing I can do ? See the "Regression Tests" section on this page: http://webkit.org/coding/contributing.html You must have a platform where layout tests are currently supported. See this page for a list of ports that currently support the tests: http://build.webkit.org/waterfall
(In reply to comment #4) > Created an attachment (id=25712) [review] > test case > > test case. click on first link, then click second and finally on third link! The test case doesn't crash for me on Safari 3.2 or with the most recent WebKit nightly build r39682. Which build of WebKit do you see this crash in?
its a very much platform dependent crash. This case was crashing in nokia mobile browser, because there is a clear corrupt/deleted node* left behind.
The reason the test case doesn't crash on Mac OS X WebKit is that navigating to a new URL results in a call to gotoAnchor in FrameLoader, and that function calls setCSSTarget(0) when you click on the second link. At that time, the object is still not deleted. I'll figure out why.
By adding a missing "preventDefault" to the test case I was able to reproduce a crash when MallocScribble is on, on Mac OS X.
Created attachment 28415 [details] test case with preventDefault added
Someone needs to turn that test case into a regression test for the layout tests directory, and then make a patch that includes both the bug fix and the test case. To make it so the test will show a crash without setting MallocScribble, at least in Debug builds, maybe we can add some code to the setChanged function in an assert. If that function called a virtual function, then I suspect we'd see a crash every time.
Comment on attachment 25670 [details] fix for empty fragment URL crash R-'ing the patch until the test case is included as Darin described.
<rdar://problem/7325983>
I cannot reproduce the crash with ToT. Actually, the code that sets CSS target to 0 was added in <http://trac.webkit.org/changeset/29311>, which was long before this bug was filed. The code was moved around, and sometimes even removed (but it seems to have been present and functional when this bug was filed). Unfortunately, the original test that came with r29311 is currently disabled, which is tracked as bug 20342.
Marking this as fixed/duplicate of 20342 as it was fixed by Yael for qtWebkit. *** This bug has been marked as a duplicate of bug 20342 ***