Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 1ea8636a1baf6f48134a8dffb6a58f607d997f3a
https://github.com/WebKit/WebKit/commit/1ea8636a1baf6f48134a8dffb6a58f607d997f3a
Author: Rupin Mittal <[email protected]>
Date: 2025-08-01 (Fri, 01 Aug 2025)
Changed paths:
M
LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/disambigaute-forward-expected.txt
M
LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/disambigaute-traverseTo-forward-multiple-expected.txt
M Source/WebCore/loader/HistoryController.cpp
M Source/WebCore/loader/NavigationScheduler.cpp
M Source/WebCore/page/Navigation.cpp
M Source/WebCore/page/Navigation.h
Log Message:
-----------
[Navigation API] navigate.back doesn't work in main frame when preceded by
navigate.back in child frame
https://bugs.webkit.org/show_bug.cgi?id=296673
rdar://157078677
Reviewed by Charlie Wolfe.
The test disambigaute-forward.html fails on this line:
"assert_equals(navigation.currentEntry.index, start_index);"
because the mainframe's back navigation fails.
This is what the test is doing:
1. Load a main frame and an iframe
2. Fragment navigate the main frame
3. Fragment navigate the iframe
4. Go back in the iframe (this succeeds)
5. Go back in the main frame (this fails to go back)
The first problem is this:
The iframe's navigation appears to succeed, but it's falsely succeeding.
Right before the iframe's back, the history tree looks like this:
1. MainFrame (URL: disambigaute-forward.html, itemID: A)
-- Child: iFrame (URL: blank.html, itemID: A)
2. MainFrame (URL: disambigaute-forward.html#top, itemID: B)
-- Child: iFrame (URL: blank.html, itemID: B)
3. MainFrame (URL: disambigaute-forward.html#top, itemID: C)
-- Child: iFrame (URL: blank.html#1, itemID: C)
When the iframe goes back, we look for the history item iFrame
(URL: blank.html, itemID: B) in the iframe's navigation's m_entries.
But m_entries contains:
iFrame (URL: blank.html, itemID: A)
iFrame (URL: blank.html#1, itemID: C)
Yet we match to item A because we are comparing by document sequence number.
The test checks that we've navigated to the item with index 0 in m_entries,
which we have. So the iframe's back navigation succeeds, but really we've
navigated to the wrong item. We should have gone to item B.
The issue here is that when the main frame navigated from A to B, that was
a "Push" navigation and we should have replaced the A entry in all of it's
children's m_entries with their new B entry. And then we should look up
items in this list by their itemID instead. So we add updateNavigationEntry()
to do this.
Now the iframe's m_entries will be this:
iFrame (URL: blank.html, itemID: B)
iFrame (URL: blank.html#1, itemID: C)
And we correctly navigate to the right item.
But our main frame's back still fails. This is the second issue:
The main frame's m_entries correctly looks like:
MainFrame (URL: disambigaute-forward.html, itemID: A)
MainFrame (URL: disambigaute-forward.html#top, itemID: B)
But on the navigate.back, the item we're looking for is wrong.
We're looking for item B when we should have been looking for item A.
Why? On navigate.back, for most of the code path, we are correctly
using item A. But then:
FrameLoader::loadInSameDocument() ends up calling
HistoryController::recursiveUpdateForSameDocumentNavigation()
which sets the currentItem to the provisionalItem. The currentItem
is the item we're looking for. Up to this point, it has correctly
been item A. But here, we set it to item B.
Turns out, the provisionalItem got set to item B by the iframe's
back navigation in HistoryController::recursiveSetProvisionalItem().
In the iframe's back, we actually call recursiveSetProvisionalItem
from the main frame's history controller. This means that we set the
provisional item for the main frame--even though the main frame is not
actually navigating. Since the main frame isn't navigating, this
provisional item does not get set to nullptr when the iframe's load
finishes. So later on, this causes the main frame's back to
look for the wrong item and for the back to fail.
So instead of calling recursiveSetProvisionalItem from the main frame's
history controller, we call it from the iframe's history controller.
We do this by calling Page::goToItemForNavigationAPI with the iframe
passed in instead of the main frame (in ScheduledHistoryNavigationByKey::fire).
This way, the provisional item won't be set for the main frame.
And since we are calling goToItemForNavigationAPI with the iframe, we
will ensure that the HistoryItems which are used (targetItem and
fromItem) also correspond to the iframe instead of the main frame
like they do currently.
I made these changes to fix disambigaute-forward.html, but it also
fixes disambigaute-traverseTo-forward-multiple.html.
*
LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/disambigaute-forward-expected.txt:
*
LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/disambigaute-traverseTo-forward-multiple-expected.txt:
* Source/WebCore/loader/HistoryController.cpp:
(WebCore::HistoryController::goToItemForNavigationAPI):
Previously, the HistoryController always used the targetItem
and fromItem of the main frame rather than the frame that owns it.
Now that it's being passed the targetItem of the owning frame, we
change it to use the fromItem of the owning frame as well. (We use
the targetItem's frameID to get the fromItem with the same frameID).
* Source/WebCore/loader/HistoryController.h:
* Source/WebCore/loader/NavigationScheduler.cpp:
(WebCore::ScheduledHistoryNavigationByKey::findBackForwardItemByKey):
This should always lookup the item in the navigating frame's list of
entries. We should not use any heuristics.
(WebCore::ScheduledHistoryNavigationByKey::fire):
This should call goToItemForNavigationAPI on the frame that is actually
navigating, not always the main frame. Accordingly, it should use the
historyItem from that navigating frame, not always the main frame.
* Source/WebCore/page/Navigation.cpp:
(WebCore::Navigation::updateNavigationEntry):
(WebCore::Navigation::updateForNavigation):
On a push navigation, we must update the subframes as well, not just
the navigating frame.
* Source/WebCore/page/Navigation.h:
Canonical link: https://commits.webkit.org/298121@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
