1636453 - Only emit Page.navigatedWithinDocument for history API usage or anchor navigation

Status: RESOLVED FIXED

(Remote Protocol :: CDP, defect, P3)

Description

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Right now we do emit this event but without any checks, so it gets emitted in all cases.

This breaks Page.waitForNavigation in Puppeteer, for example, where we see a race with Network events.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The hashchange event listener should help here. At least for the first part of the event definition:

Fired when same-document navigation happens, e.g. due to history API usage or anchor navigation.

Whereby history API usage is related to Page.navigateToHistoryEntry (bug 1609152).

So we might want to fix both at once.

Comment 2

[Julian Descottes [:jdescottes]]

Related to https://github.com/puppeteer/puppeteer/issues/8361

Comment 3

[Julian Descottes [:jdescottes]]

Trying to move navigatedWithinDocument to hashchange breaks a few navigations with the current implementation of puppeteer. See for more details, but the summary is that to do this we need to first make sure that loaderId is correctly provided for frameNavigated events, even for navigations which don't hit the network.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1636453 - [cdp] Move Page.navigatedWithinDocument to hashChange

Depends on D146984

This is needed to be forward compatible with new versions of puppeteer
The navigatedWithinDocument event should only have been emitted for same
document navigation (either hashchange or history navigation), but it was
emitted on load.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1636453 - [cdp] Support Page.navigate for hash navigation

Depends on D146985

The previous changeset changed the implementation of navigatedWithinDocument to
be emitted in the proper scenarios (ie, same document navigation).

However our Page.navigate implementation would still timeout waiting on a
network request if you tried to programmatically perform a hash navigation.
Meaning hash navigation was only working when triggered from the page.

With this changeset, we try to detect hash navigations in Page.navigate and
treat them as "network-less" navigations (same as for non http/https navigations)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0133613140be
[cdp] Move Page.navigatedWithinDocument to hashChange r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/ad7a8c63d820
[cdp] Support Page.navigate for hash navigation r=webdriver-reviewers,whimboo

Comment 7

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/0133613140be
https://hg.mozilla.org/mozilla-central/rev/ad7a8c63d820