Created attachment 58861 [details] Modified MiniBrowser to demonstrate problem After r61207 calls to -[WebView goToBackForwardItem:] do not appear to initiate loads. I’ve attached a version of Apple’s MiniBrowser sample code that uses -goToBackForwardItem: to perform the initial load. When run against shipping WebKit it loads apple.com as expected. When run against r61207 the WebView remains empty. <rdar://problem/8097740>
Created attachment 58862 [details] Diff showing modification to MiniBrowser
The key change can be seen here: http://trac.webkit.org/changeset/61207#file10 line 553 In HistoryController::recursiveGoToItem() // If the item we're going to is a clone of the item we're at, then do // not load it again, and continue history traversal to its children. // The current frame tree and the frame tree snapshot in the item have // to match. if (item->itemSequenceNumber() == fromItem->itemSequenceNumber() ... // do some stuff with subframes but don't load in the current frame In the minibrowser example attached, the WebView is navigated to the same historyitem. So, of course, the item and fromItem both have the same document sequence number because they are the same item. Even if it wasn't explicit, the previous behavior was definitely to force the load when going to the same item. We need to restore that.
FrameLoader::loadItem has a check that item != currentItem. It uses that to determine that navigateToDifferentDocument should be called instead of navigateWithinDocument. We can certainly replicate that same check in HistoryController::recursiveGoToItem. It seems logically that they should be similar.
Notice that ScheduledHistoryNavigation (in RedirectScheduler.cpp) special cases go(0) to call FrameLoader::urlSelected instead of Page::goBackOrForward. This explains why we don't see any problems with history.go(0). Anyways, I'll put together the patch to HistoryController.cpp as mentioned above.
(In reply to comment #3) > FrameLoader::loadItem has a check that item != currentItem. It uses that to determine that navigateToDifferentDocument should be called instead of navigateWithinDocument. We can certainly replicate that same check in HistoryController::recursiveGoToItem. It seems logically that they should be similar. I agree FrameLoader::loadItem has that check, but the code path we're talking about in this scenario is: #0 0x10295eb7a in WebCore::HistoryController::recursiveGoToItem at HistoryController.cpp:537 #1 0x10295f024 in WebCore::HistoryController::goToItem at HistoryController.cpp:223 #2 0x102dc2410 in WebCore::Page::goToItem at Page.cpp:315 #3 0x1020d61a0 in -[WebView goToBackForwardItem:] at WebView.mm:3174
(In reply to comment #5) > (In reply to comment #3) > > FrameLoader::loadItem has a check that item != currentItem. It uses that to determine that navigateToDifferentDocument should be called instead of navigateWithinDocument. We can certainly replicate that same check in HistoryController::recursiveGoToItem. It seems logically that they should be similar. > > I agree FrameLoader::loadItem has that check, but the code path we're talking about in this scenario is: > > > #0 0x10295eb7a in WebCore::HistoryController::recursiveGoToItem at HistoryController.cpp:537 > #1 0x10295f024 in WebCore::HistoryController::goToItem at HistoryController.cpp:223 > #2 0x102dc2410 in WebCore::Page::goToItem at Page.cpp:315 > #3 0x1020d61a0 in -[WebView goToBackForwardItem:] at WebView.mm:3174 Sorry, I misread your original comment. You seemed to agree completely that this path should be similar. Thanks! :)
(In reply to comment #6) > Sorry, I misread your original comment. You seemed to agree completely that this path should be similar. Posting a patch shortly. Sorry for not being able to get to it yesterday.
Created attachment 59007 [details] v1 patch Unfortunately, this bug cannot be replicated with HTML+JS, so I have not included any kind of regression test.
Comment on attachment 59007 [details] v1 patch Looks good. No worries for not being yesterday, I appreciate the fix!
Landed as http://trac.webkit.org/changeset/61335