RESOLVED DUPLICATE of bug 20342 22590
empty fragment href's crashes browser.
https://bugs.webkit.org/show_bug.cgi?id=22590
Summary empty fragment href's crashes browser.
Mahesh Kulkarni
Reported 2008-12-02 08:30:11 PST
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.
Attachments
fix for empty fragment URL crash (1019 bytes, patch)
2008-12-02 08:44 PST, Mahesh Kulkarni
adele: review-
test case (347 bytes, text/html)
2008-12-03 09:29 PST, Mahesh Kulkarni
no flags
test case with preventDefault added (371 bytes, text/html)
2009-03-09 09:57 PDT, Darin Adler
no flags
Mahesh Kulkarni
Comment 1 2008-12-02 08:44:37 PST
Created attachment 25670 [details] fix for empty fragment URL crash assigning m_cssTarget to null if that node is getting deleted.
Antti Koivisto
Comment 2 2008-12-02 13:38:05 PST
Could you provide a test case as well?
Mark Rowe (bdash)
Comment 3 2008-12-02 16:02:09 PST
*** Bug 22591 has been marked as a duplicate of this bug. ***
Mahesh Kulkarni
Comment 4 2008-12-03 09:29:22 PST
Created attachment 25712 [details] test case test case. click on first link, then click second and finally on third link!
Darin Adler
Comment 5 2008-12-04 09:32:12 PST
Comment on attachment 25670 [details] fix for empty fragment URL crash The fix looks fine. Now we need a regression test for this.
Mahesh Kulkarni
Comment 6 2008-12-11 23:33:32 PST
Please let me know what is the next thing I can do ?
David Kilzer (:ddkilzer)
Comment 7 2008-12-12 04:47:15 PST
(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
David Kilzer (:ddkilzer)
Comment 8 2009-01-07 19:22:55 PST
(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?
Mahesh Kulkarni
Comment 9 2009-01-22 04:15:12 PST
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.
Darin Adler
Comment 10 2009-03-09 09:51:12 PDT
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.
Darin Adler
Comment 11 2009-03-09 09:56:56 PDT
By adding a missing "preventDefault" to the test case I was able to reproduce a crash when MallocScribble is on, on Mac OS X.
Darin Adler
Comment 12 2009-03-09 09:57:28 PDT
Created attachment 28415 [details] test case with preventDefault added
Darin Adler
Comment 13 2009-03-09 10:00:35 PDT
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.
Adele Peterson
Comment 14 2009-03-23 11:15:10 PDT
Comment on attachment 25670 [details] fix for empty fragment URL crash R-'ing the patch until the test case is included as Darin described.
Alexey Proskuryakov
Comment 15 2009-10-22 00:11:33 PDT
Alexey Proskuryakov
Comment 16 2009-10-23 10:09:06 PDT
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.
Mahesh Kulkarni
Comment 17 2011-03-08 11:57:31 PST
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 ***
Note You need to log in before you can comment on or make changes to this bug.