New issue
Advanced search Search tips

Issue 866406 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Fix testDoubleBackJSNavigation

Project Member Reported by olivierrobin@chromium.org, Jul 23

Issue description

This test is not correct

It loads URL1, URL2, URL3,
Then in page 3 calls twice window.history.back and expect to see URL1
The WindowId both messages is the windowID of page3

Calling Javascript in URL3 should not bypass URL2

 
Cc: gambard@chromium.org
Owner: eugene...@chromium.org
This is not a #slim-navigation-manager specific bug. Assigning to eugenebut@ to track with general navigation issues.
Olivier, why do you expect URL2 to be loaded after double back navigation? First "back" starts the back navigation, which never finishes because the server is paused. Second "back" cancels the first navigation and starts the new one. This is how WKWebView works and the test verifies that.

What is the context for this bug? I.e. what is the underlining issue behind it?
The navigation is done in JS.
The first window.history.back triggers a navigation away from the page, so this should destroy the JS context.
This should also update the window ID to filter out the second message, but the current implementation only updates the window ID on page load complete.

The second window.history.back should not be executed.

A small question to highlight my point. Imagine that page2 loading is not blocked.
Page3 sends 2 window.history.back events.
Should the page3 be allowed to execute both events?
What if page2 immediately sends window.history.forward?

The problem happened when I tried to implement filtering by frameID.
I clear my frameIDs on navigation, so the second message is ignored and only one navigation happens,

And I think that even if the back() is executed twice, it should trigger two navigations to the previous entry in history and load twice page2, and not load page1.
I investigated a little more.
I created
https://www.r0b.in/test/page1.htm

Then click on next, it goes to page2
Then click on next, it goes to page3
Then click on test.

on Chrome, (iOS and desktop), it goes to page1.
on any other browser tested (opera, firefox, safari, on iOS, Android and mac), it goes to page2.

By HTML sppecification
https://html.spec.whatwg.org/multipage/history.html#traverse-the-history-by-a-delta
it seems that chrome behavior is a bug, because


step 1 specifies that the resolution of the target index must be done on the current entry, 
step 5.1 specifies that if there is a pending navigation, it must be cancelled (->there should be only one navigation)
>>> I clear my frameIDs on navigation, so the second message is ignored and only one navigation happens,
This could be an issue with legacy navigation system which relies on JS overrides. Do you see the same problem when slim-navigation-manager is enabled?

Comment 8 Deleted

I deleted my previous comment and posting revised version.

Re to comment #6:

Thanks a lot for digging this up. The spec says the following: "If there is an ongoing attempt to navigate specified browsing context that has not yet matured (i.e. it has not passed the point of making its Document the active document), then cancel that attempt to navigate the browsing context."

Sounds like the first back navigation should be cancelled, which means that only the second navigation should succeed and Chrome does not follow the spec. This should be fixed with slim-navigation-manager enabled. Fixing this bug in legacy navigation system will be very hard.
Cc: danyao@chromium.org
Reported crbug.com/869710 against Desktop
I interpret it the same way. 
I added another test that start at https://www.r0b.in/test/page_b1.htm
and does a 
window.history.go(-1);window.history.go(-2);
on page 4.

All browser go to page2, except chrome that goto page 1.

This also mean that clearing FrameID on navigation triggered is incorrect.
I am not really an expert on the navigation stack. 
Do you think clearing them on navigationCommitted is enough?
Apparently I'm not an expert on the navigation stack either :). But I think clearing frames on didCommitNavigation should be enough. 
Cc: michaeldo@chromium.org
Labels: -Pri-2 Pri-3
The test is fixed with web::features::kWebFrameMessaging enabled. There is still a TODO in the code, which can be removed when kWebFrameMessaging flag is cleaned up.
Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment