Add a browsingContext.traverseHistory command #109

jgraham · 2021-06-04T14:32:31Z

This traverses the history by a specified delta (e.g. +1 for forward,
or -1 for back). Like other navigation commands the wait parameter
controls whether to return as soon as the history traversal starts, or
wait for a specified level of completeness.

Mush like other navigation commands, history traversal is tracked with
a unique navigation id.

Typically history traversal results in a navigation, but in the case
of the history entry being in the bfcache, no navigation is
necessary. In this case any non-"none" value for wait will return once
the pageshow event is fired, and the persisted attribute of the
return value is set to true.

This model differs somewhat from the CDP model where one navigates to
an explicit history entry and failure is only possible if the entry id
is invalid. But back/forwward only seems closer to the use cases we
have, and allowing events to be traced to a specific traversal seems
consistent with the way we handle other navigation-related events.

Preview | Diff

jgraham · 2021-06-04T14:33:29Z

This depends on #107 plus some additional changes to the HTML spec that I haven't created a PR for yet.

whimboo · 2021-06-07T08:27:32Z

index.bs

+
+      BrowsingContextReloadParameters = {
+        context: BrowsingContext,
+        delta: int,


Using a delta to define the number of history entries to move back or forward means that we also need an API to actually retrieve the maximum amount of entries.

Maybe pages might also be interested in to go back/forward to a specific URL.

It doesn't mean we need that if there aren't use cases for it. HTTP-based WebDriver just has /back and /forward. Puppeteer has goBack and goForward, which PlayWright seems to have kept unchanged. Cypress has go which takes a delta. I can't see any of them having an API to go back/forward to a specific URL or to get a the size of the session history.

I think if those use cases come up later we'd be better off adding an API that returns a list of session history entries, like CDP. Such an PI seems useful for testing the platform itself, but it's hard to argue it's part of the MVP for replacing existing automation tools given none of them provide it.

whimboo · 2021-06-07T08:28:55Z

index.bs

+     parameters| if present, or "<code>none</code>" otherwise.
+
+  1. Let |navigate status| be the result of running the [=traverse the history
+     by a delta steps=] given |delta|, and |context|.


What should happen if delta is outside of the maximum available pages in the history? Should it be silently ignored as in this case, or a specific error raised?

There will be an error although at this point there's a lot of unknown error. I (or someone else!) need to put some time into rewriting the error codes for BiDi.

@jgraham would you mind filing a new issue for that with details? Would be good to keep that on our radar.

#112 covers this.

whimboo · 2021-06-07T08:30:40Z

index.bs

+ 1. If |event received| is "<code>navigation failed</code>"
+    return [=error=] with [=error code=] [=unknown error=].
+
+ 1. If |event received| is "<code>page show</code>", let |persisted| be true,


Shouldn't this be set to the value of the persisted property of pageshow?

Not as currently written because the hook in the HTML spec (which needs a seperate PR) is only invoked if the page came out of the bfcache, not if the page was loaded normally. Therefore persisted is always true in this case. If we change things so there's a BiDi context.pageShow event then we'll need to set the persisted property in the hook and reflect that here.

whimboo · 2021-06-07T08:54:54Z

index.bs

+The <dfn export>WebDriver-BiDi page show</dfn> steps given <var
+ignore>context</var> and |navigation status| are:
+
+Issue: Do we want to expose a `browsingContext.pageShow event? In that case we'd


Isn't that what we will do for navigation anyway when no wait parameter has been given and the command returns immediately?

The question is whether we want a specific event for it so that clients can wait for pageShow even if it's triggered by something other than a navigation command. I'm inclined to think it would probably be a good idea, given pageshow/pagehide are typically the most reliable things to use in the face of bfcache. I don't quite know if it's covered by Page.lifecycleEvent in CDP since the docs don't mention when the events are actually emitted.

css-meeting-bot · 2021-06-09T16:30:59Z

The Browser Testing and Tools Working Group just discussed context.traverseHistory.

The full IRC log of that discussion

<jgraham> topic: context.traverseHistory
<AutomatedTester> github: https://github.com//pull/109
<AutomatedTester> jgraham (IRC): The third change is moving through history
<AutomatedTester> ... I have a draft PR for the html spec changes we need
<AutomatedTester> ... the model here is that you can move via a delta
<AutomatedTester> ... this is different to CDP but the clients do this
<AutomatedTester> ... and it is closer to what cypress does as other tools just have back/forward
<AutomatedTester> ... and since there are things in the bfcache there might not be events that are returned to the user
<AutomatedTester> q?

WebDriver BiDi wants to invoke the 'traverse the history by a delta` algorithm, and get a callback whenever the algoritihm has run to completion, either by failing or by the navigation or state restoration completing. w3c/webdriver-bidi#109 is the WebDriver BiDi side of this change.

jgraham · 2021-07-30T20:22:46Z

whatwg/html#6921 for the HTML side of this change.

WebDriver BiDi wants to invoke the 'traverse the history by a delta` algorithm, and get a callback whenever the algoritihm has run to completion, either by failing or by the navigation or state restoration completing. w3c/webdriver-bidi#109 is the WebDriver BiDi side of this change.

bwalderman

LGTM. Just left some minor comments.

bwalderman · 2021-09-21T20:23:47Z

index.bs

+1. Assert: |navigate status|'s status is "<code>pending</code>" and
+   |navigation id| is not null.
+
+1. If |wait condition| is "<code>none</code>":


wait condition needs to be passed as a parameter to this algorithm.

index.bs

foolip · 2021-09-23T07:23:12Z

index.bs

+     <code>url</code> field set to the result of the [=URL serializer=] given
+     |navigate status|'s url.
+
+  1. Return [=success=] with data |body|, and then run the following steps [=in


Will the "in parallel" here be replaced with "queue a task" after #115?

index.bs

foolip · 2021-09-23T07:24:35Z

index.bs

+  1. Return [=success=] with data |body|, and then run the following steps [=in
+     parallel=]:
+
+     1. Run the [=WebDriver-BiDi fragment navigated=] steps given |context|


Matching whatwg/html#6921:

Suggested change

1. Run the [=WebDriver-BiDi fragment navigated=] steps given |context|

1. Run the [=WebDriver BiDi fragment navigated=] steps given |context|

foolip · 2021-09-23T07:24:41Z

index.bs

+  1. Return [=success=] with data |body|, and then run the following steps [=in
+     parallel=]:
+
+     1. Run the [=WebDriver-BiDi navigation started=] steps given |context|


This is all a bit hard to grasp initially, so to spell out what's going on with all the returning and in parallel: it's to ensure that the command response comes before any events. The "WebDriver BiDi navigation started" algorithm just emits the event, it doesn't call into HTML or anything else.

Also good ol' hyphen suggestion:

Suggested change

1. Run the [=WebDriver-BiDi navigation started=] steps given |context|

1. Run the [=WebDriver BiDi navigation started=] steps given |context|

index.bs

foolip · 2021-09-23T07:31:07Z

index.bs

+     1. Run the [=WebDriver-BiDi fragment navigated=] steps given |context|
+        and |navigate status|
+
+   Note: this is the case if the navigation only caused the fragment to


I'm not sure I follow what's going on here. This happens unconditionally, but history traversal isn't always a fragment navigation. Does this perfectly and intricately balance the now conditional invocation of "WebDriver BiDi fragment navigated" on the HTML side that I wondered about in whatwg/html#6921 (comment)?

Also, capitalization nit since the note is forever:

Suggested change

Note: this is the case if the navigation only caused the fragment to

Note: This is the case if the navigation only caused the fragment to

foolip · 2021-09-23T07:34:44Z

index.bs

+1. Run the [=WebDriver-BiDi navigation started=] steps given |context|
+   and |navigate status|
+
+   Note: this event was previously suppressed to ensure that it would come


Closing the door to my initial reading that this is about a previous state of the spec:

Suggested change

Note: this event was previously suppressed to ensure that it would come

Note: This event was previously suppressed to ensure that it comes

foolip · 2021-09-23T07:35:23Z

index.bs

+   after the command response in the case that |wait condition| is
+   "<code>none</code>".
+
+   Issue: Replace this suppression mechanism with an event queue.


Yes please 👍

foolip · 2021-09-23T07:42:00Z

index.bs

+
+</div>
+
+#### The browsingContext.reload Command ####  {#command-browsingContext-reload}


What's the browsingContext.reload addition doing here? I assume it'll go away with a rebase and will ignore here.

WebDriver BiDi wants to invoke the 'traverse the history by a delta` algorithm, and get a callback whenever the algoritihm has run to completion, either by failing or by the navigation or state restoration completing. w3c/webdriver-bidi#109 is the WebDriver BiDi side of this change.

whimboo · 2022-04-29T05:38:22Z

Hi @jgraham. I just stumbled over this PR and wonder what's actually left to do here. There are some review comments you might wanna have a look at.

jgraham · 2022-04-29T08:42:38Z

Yeah, this got dropped a bit. The main thing left to do is actually land the HTML side patch.

foolip · 2023-02-23T17:08:37Z

@jgraham I've merged to resolve conflicts, but further changes are still needed. The "child browsing contexts" concept is no longer in HTML, and perhaps other things.

This traverses the history by a specified `delta` (e.g. +1 for forward, or -1 for back). Like other navigation commands the `wait` parameter controls whether to return as soon as the history traversal starts, or wait for a specified level of completeness. Mush like other navigation commands, history traversal is tracked with a unique navigation id. Typically history traversal results in a navigation, but in the case of the history entry being in the bfcache, no navigation is necessary. In this case any non-"none" value for wait will return once the `pageshow` event is fired, and the `persisted` attribute of the return value is set to `true`. This model differs somewhat from the CDP model where one navigates to an explicit history entry and failure is only possible if the entry id is invalid. But back/forwward only seems closer to the use cases we have, and allowing events to be traced to a specific traversal seems consistent with the way we handle other navigation-related events.

whimboo · 2023-08-24T18:56:26Z

index.bs

  browsingContext.SetViewport
+  browsingContext.TraverseHistory


Suggested change

browsingContext.SetViewport

browsingContext.TraverseHistory

browsingContext.SetViewport //

browsingContext.TraverseHistory

whimboo · 2023-08-24T18:56:41Z

index.bs

@@ -1864,6 +1866,7 @@ BrowsingContextResult = (
  browsingContext.GetTreeResult //
  browsingContext.NavigateResult //
  browsingContext.PrintResult


Suggested change

browsingContext.PrintResult

browsingContext.PrintResult //

whimboo · 2023-08-24T18:57:54Z

index.bs

+
+      browsingContext.TraverseHistoryParameters = {
+        context: browsingContext.BrowsingContext,
+        delta: int,


Suggested change

delta: int,

delta: js-int,

whimboo · 2023-08-24T18:58:58Z

index.bs

+            ?persisted: bool
+            url: text,


Suggested change

?persisted: bool

url: text,

url: text,

? persisted: bool

whimboo · 2023-08-24T19:04:54Z

index.bs

+1. Let |browsing context| be the result of [=trying=] to [=get a browsing context=]
+   with |command parameters|["<code>context</code>"]|.
+


whimboo · 2023-08-24T19:10:14Z

index.bs

+   condition| be |command parameters|["<code>wait</code>"]. Otherwise let |wait
+   condition| be "<code>none</code>".
+
+1. Let |navigation id| be the string representation of a


Could we maybe integrate all the following in await a navigation? We may be able to pass in another argument like delta to differentiate between a normal navigation and a history traversal. That way we wouldn't have to duplicate that much code.

whimboo · 2023-08-24T19:12:13Z

index.bs

+
+Issue: Do we want to expose a `browsingContext.pageShow event? In that case we'd
+need to call this whenever `pageshow` is going to be emitted, not just on
+bfcache restore, and also add the persisted status to the data.


@OrKoN would that be beneficial / helpful for Puppeteer?

I think it might be but we currently do not support bfcache in Puppeteer. I think it's related to the prerendering topic too #321 So far there is nothing in WebDriver BiDi that would cover navigations that are bfcache restore/prerendering activation. I think we will need to come up with something before restoring would result in a different browsing context becoming the primary context in a tab.

whimboo · 2023-10-30T17:36:02Z

Superseded by #584?

jgraham · 2023-10-30T17:38:02Z

Not quite, let's move this one back to draft and it can cover the addition of wait and a navigation id.

jgraham force-pushed the navigate_history branch from 297039e to b99f1ed Compare June 4, 2021 14:35

whimboo reviewed Jun 7, 2021

View reviewed changes

jgraham force-pushed the navigate_reload branch from 5812fae to 9771433 Compare July 14, 2021 19:55

jgraham marked this pull request as ready for review July 14, 2021 19:57

jgraham marked this pull request as draft July 14, 2021 19:59

jgraham force-pushed the navigate_history branch 2 times, most recently from 39072b4 to 3832f2a Compare July 30, 2021 20:17

jgraham marked this pull request as ready for review July 30, 2021 20:21

jgraham mentioned this pull request Jul 30, 2021

Add navigation hooks for WebDriver BiDi history traversal whatwg/html#6921

Open

jgraham force-pushed the navigate_reload branch from 9771433 to ea398f9 Compare September 14, 2021 19:34

Base automatically changed from navigate_reload to master September 15, 2021 10:22

whimboo added module-browsingContext Browsing Context module enhancement New feature or request labels Sep 20, 2021

bwalderman approved these changes Sep 21, 2021

View reviewed changes

foolip reviewed Sep 23, 2021

View reviewed changes

jgraham force-pushed the navigate_history branch from 8e3c428 to c75510b Compare September 23, 2021 14:24

This was referenced Dec 1, 2022

Don't break lines in some blocks #208

Open

Add type field to "ScriptEvaluateResult" for "success" or "exception" #340

Merged

jgraham added 3 commits July 28, 2023 11:45

Fix typos

fd2ce0a

Update to match current spec

58fccb9

jgraham force-pushed the navigate_history branch from acf3b05 to 58fccb9 Compare July 28, 2023 11:13

whimboo requested changes Aug 24, 2023

View reviewed changes

jgraham marked this pull request as draft October 30, 2023 17:38

OrKoN mentioned this pull request Nov 8, 2023

Implement traverseHistory GoogleChromeLabs/chromium-bidi#1532

Closed

	1. Run the [=WebDriver-BiDi fragment navigated=] steps given \|context\|
	1. Run the [=WebDriver BiDi fragment navigated=] steps given \|context\|

	1. Run the [=WebDriver-BiDi navigation started=] steps given \|context\|
	1. Run the [=WebDriver BiDi navigation started=] steps given \|context\|

	Note: this is the case if the navigation only caused the fragment to
	Note: This is the case if the navigation only caused the fragment to

	Note: this event was previously suppressed to ensure that it would come
	Note: This event was previously suppressed to ensure that it comes


		</div>

		#### The browsingContext.reload Command #### {#command-browsingContext-reload}

		1. Let \|browsing context\| be the result of [=trying=] to [=get a browsing context=]
		with \|command parameters\|["<code>context</code>"]\|.

-. Let |browsing context| be the result of [=trying=] to [=get a browsing context=]
-   with |command parameters|["<code>context</code>"]|.
+. Let |context id| be the value of the <code>context</code> field of
+   |command parameters|.
+. Let |context| be the result of [=trying=] to [=get a browsing context=]
+   with |context id|.

Add a browsingContext.traverseHistory command #109

Are you sure you want to change the base?

Add a browsingContext.traverseHistory command #109

Conversation

jgraham commented Jun 4, 2021 • edited by pr-preview bot Loading

jgraham commented Jun 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

css-meeting-bot commented Jun 9, 2021

jgraham commented Jul 30, 2021

bwalderman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whimboo commented Apr 29, 2022

jgraham commented Apr 29, 2022

foolip commented Feb 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OrKoN Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

whimboo commented Oct 30, 2023

jgraham commented Oct 30, 2023

jgraham commented Jun 4, 2021 •

edited by pr-preview bot

Loading

OrKoN Aug 25, 2023 •

edited

Loading