Going back from the 'All Sizes' page of a photo on Flickr no longer works. 1. In Safari, open a new tab and load http://www.flickr.com/photos/n1c0star/538230787/ 2. Click the "All Sizes" button above the photo. 3. The back button is disabled and no history item created. Bug is present in Safari 3 Beta and r23521. Note that this is not an issue with Safari.app as Safari.app v2.0.4 with current ToT displays this bug.
<rdar://problem/5270940>
Created attachment 15137 [details] proposed patch The call for scheduleLocationChange() in JSHTMLDocument::setLocation() (JSHTMLDocumentCustom.cpp) was missing an argument (userGesture) and the default behavior in scheduleLocationChange() was to consider it false. As this JS event was not considered as userGesture initiated, his load type was internal and it wasn't added to the history. With this modification I can no longer reproduce the bug
Layout test is on his way. I'll send it within the next hours.
Created attachment 15159 [details] patch with layout test Here is the patch with the layout test included. I noticed that this patch is also fixing the Bug 13849.
Comment on attachment 15159 [details] patch with layout test r=me
*** Bug 13849 has been marked as a duplicate of this bug. ***
Created attachment 15184 [details] patch with correct layout test The layout test wasn't correctly working due to a race condition. Now it's works as it should.
Comment on attachment 15159 [details] patch with layout test Setting r- so it doesn't show up in commit queue.
Comment on attachment 15184 [details] patch with correct layout test I went to land this patch but noticed that the test case it adds causes an assertion failure to be hit. $ run-webkit-tests LayoutTests/http/tests/navigation/javascript-document-location-changed.html LayoutTests/http/tests/navigation/javascript-document-location-changed.html [...] http/tests/navigation ..Assertion failed: ([bfList currentItem] != prevTestBFItem), function dumpBackForwardListForWebView, file /Volumes/Data/Home/Documents/Work/OpenSource/WebKitTools/DumpRenderTree/DumpRenderTree.m, line 768. I hit this while regenerating the test results after removing the tab characters, fixing spelling, and changing the test to dump as text.
Created attachment 15243 [details] proposed patch I can't reproduce the assertion failure you hitted, so I made some changes without being able to test the final result. I hope this will be good. And I'm sorry for the bad spelling : I'm french and I'm trying very hard to create correct sentences, I hope I will improve my writing within the next 4 months.
(In reply to comment #10) > Created an attachment (id=15243) [edit] > proposed patch > > I can't reproduce the assertion failure you hitted, so I made some changes > without being able to test the final result. > I hope this will be good. > And I'm sorry for the bad spelling : I'm french and I'm trying very hard to > create correct sentences, I hope I will improve my writing within the next 4 > months. > I'm hitting the same assertion failure in your latest patch. Are you sure you're using a debug build?
Comment on attachment 15243 [details] proposed patch r- due to assertion failures.
Created attachment 15271 [details] proposed patch I changed the whole test to be sure to avoid the assertion failure. I think it's for the best because the test is now faster and more simple.
I no longer see the assertion failure. The question I have is why was the assertion failing, and why does it not fail with your new test?
(In reply to comment #14) > I no longer see the assertion failure. The question I have is why was the > assertion failing, and why does it not fail with your new test? > I think it's because the test was based on a simulated clic to go to the next page instead of LayoutTestController.queueLoad(). So when I came back to the first page, the LayoutTestController.dumpBackForwardList() was called again. In the new test I clear the backForwardList on the first page and, when I'm on the second page, instead of going back to the first page, I'm analysing the history to see if it's there.
Comment on attachment 15271 [details] proposed patch r=me
Comment on attachment 15271 [details] proposed patch It seems that JSHTMLDocument::setLocation has been changed since you created the patch, so the patch no longer applies cleanly. It's not obvious to me how to reconcile the changes so it would be great if you could do so and submit an updated patch.
This bug is now fixed by the patch of Bug 13400 which is a duplicate of this one
*** This bug has been marked as a duplicate of 13400 ***