Bug 282689 – [Navigation] Fix remaining problems with traversals

• Component: Page Loading
• Status: NEW

[Rob Buis]

Fix remaining problems with traversals.

[Rob Buis]

Traversal tests timing out:

navigation-methods/back-forward-multiple-frames.html
traverseTo-detach-between-navigate-and-navigatesucces.html
navigation-traverseTo-in-iframe-same-document-preventDefault.html
navigation-traverseTo-navigates-top-and-same-doc-child-and-cross-doc-child.html

[Rob Buis]

This will have to be debugged more closely to see what is the problem. I suspect that for at least for one of them the problem is not correctly cancelling navigation for both top and subframe.

[Radar WebKit Bug Importer]

<rdar://problem/139799959>

[Chris Dumez]

I started taking a look at navigation-methods/back-forward-multiple-frames.html.

At the following step: // Going back in top should go 3->1 (navigating both windows).
This resolves:
```
await navigation.back().commited;
```

but this doesn't:
```
new Promise(resolve => i.contentWindow.onpopstate = resolve)
```

[Chris Dumez]

navigation-methods/back-forward-multiple-frames.html has a top frame and an iframe.
The test does this:
1. Navigate the top frame to "#top"
2. Navigate the iframe to "#iframe"
3. Call `navigation.back()` in the top frame
4. Expects both the top frame and the iframe to get navigated back (so that the fragment is removed from both) and a popstate event to get fired in both frames.

This works in Blink but in WebKit, only the top frame gets navigated and gets a popstate event. The iframe is still sitting on "#iframe" after calling `navigation.back()` in the top frame.

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> navigation-methods/back-forward-multiple-frames.html has a top frame and an
> iframe.
> The test does this:
> 1. Navigate the top frame to "#top"
> 2. Navigate the iframe to "#iframe"
> 3. Call `navigation.back()` in the top frame
> 4. Expects both the top frame and the iframe to get navigated back (so that
> the fragment is removed from both) and a popstate event to get fired in both
> frames.
> 
> This works in Blink but in WebKit, only the top frame gets navigated and
> gets a popstate event. The iframe is still sitting on "#iframe" after
> calling `navigation.back()` in the top frame.

Looking at the `Navigation::back()` implementation, it does look like it doesn't quite match the specification and only navigates a single frame. In the spec, we're supposed to also navigate child frames if the navigation of the parent frame doesn't change document (i.e. just a fragment navigation).

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #5)
> > navigation-methods/back-forward-multiple-frames.html has a top frame and an
> > iframe.
> > The test does this:
> > 1. Navigate the top frame to "#top"
> > 2. Navigate the iframe to "#iframe"
> > 3. Call `navigation.back()` in the top frame
> > 4. Expects both the top frame and the iframe to get navigated back (so that
> > the fragment is removed from both) and a popstate event to get fired in both
> > frames.
> > 
> > This works in Blink but in WebKit, only the top frame gets navigated and
> > gets a popstate event. The iframe is still sitting on "#iframe" after
> > calling `navigation.back()` in the top frame.
> 
> Looking at the `Navigation::back()` implementation, it does look like it
> doesn't quite match the specification and only navigates a single frame. In
> the spec, we're supposed to also navigate child frames if the navigation of
> the parent frame doesn't change document (i.e. just a fragment navigation).

Relevant spec: https://html.spec.whatwg.org/#getting-all-navigables-that-might-experience-a-cross-document-traversal

[Chris Dumez]

(In reply to Chris Dumez from comment #7)
> (In reply to Chris Dumez from comment #6)
> > (In reply to Chris Dumez from comment #5)
> > > navigation-methods/back-forward-multiple-frames.html has a top frame and an
> > > iframe.
> > > The test does this:
> > > 1. Navigate the top frame to "#top"
> > > 2. Navigate the iframe to "#iframe"
> > > 3. Call `navigation.back()` in the top frame
> > > 4. Expects both the top frame and the iframe to get navigated back (so that
> > > the fragment is removed from both) and a popstate event to get fired in both
> > > frames.
> > > 
> > > This works in Blink but in WebKit, only the top frame gets navigated and
> > > gets a popstate event. The iframe is still sitting on "#iframe" after
> > > calling `navigation.back()` in the top frame.
> > 
> > Looking at the `Navigation::back()` implementation, it does look like it
> > doesn't quite match the specification and only navigates a single frame. In
> > the spec, we're supposed to also navigate child frames if the navigation of
> > the parent frame doesn't change document (i.e. just a fragment navigation).
> 
> Relevant spec:
> https://html.spec.whatwg.org/#getting-all-navigables-that-might-experience-a-
> cross-document-traversal

And https://html.spec.whatwg.org/#get-all-navigables-whose-current-session-history-entry-will-change-or-reload

[Chris Dumez]

Still failing:
- imported/w3c/web-platform-tests/navigation-api/navigation-methods/traverseTo-detach-between-navigate-and-navigatesuccess.html
- imported/w3c/web-platform-tests/navigation-api/navigate-event/navigation-traverseTo-in-iframe-same-document-preventDefault.html

Other failures have been addressed in the dependency Bug 284865.

[Chris Dumez]

Bug 285045 should take care of:
- imported/w3c/web-platform-tests/navigation-api/navigate-event/navigation-traverseTo-in-iframe-same-document-preventDefault.html

[Chris Dumez]

Did some early investigation for imported/w3c/web-platform-tests/navigation-api/navigation-methods/traverseTo-detach-between-navigate-and-navigatesuccess.html, it seems due to the following code in `FrameLoader::stopLoading()`:
```
        if (document->settings().navigationAPIEnabled() && !m_doNotAbortNavigationAPI && unloadEventPolicy != UnloadEventPolicy::UnloadAndPageHide) {
            RefPtr window = m_frame->document()->domWindow();
            window->protectedNavigation()->abortOngoingNavigationIfNeeded();
        }
```

If you comment out the `unloadEventPolicy != UnloadEventPolicy::UnloadAndPageHide` check then the test start passing.

[Chris Dumez]

(In reply to Chris Dumez from comment #11)
> Did some early investigation for
> imported/w3c/web-platform-tests/navigation-api/navigation-methods/traverseTo-
> detach-between-navigate-and-navigatesuccess.html, it seems due to the
> following code in `FrameLoader::stopLoading()`:
> ```
>         if (document->settings().navigationAPIEnabled() &&
> !m_doNotAbortNavigationAPI && unloadEventPolicy !=
> UnloadEventPolicy::UnloadAndPageHide) {
>             RefPtr window = m_frame->document()->domWindow();
>             window->protectedNavigation()->abortOngoingNavigationIfNeeded();
>         }
> ```
> 
> If you comment out the `unloadEventPolicy !=
> UnloadEventPolicy::UnloadAndPageHide` check then the test start passing.

Making this change breaks a few tests though:
  imported/w3c/web-platform-tests/navigation-api/navigation-methods/navigate-history-push-same-url-cross-document.html [ Failure ]
  imported/w3c/web-platform-tests/navigation-api/navigation-methods/return-value/traverseTo-detach-same-document-before-navigate-event.html [ Failure ]
  imported/w3c/web-platform-tests/navigation-api/ordering-and-transition/transition-cross-document.html [ Failure ]