Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail aborted navigation #800

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented Oct 23, 2024

@whimboo
Copy link
Contributor

whimboo commented Oct 24, 2024

I would still like to see a debate if from a user perspective this is a acceptable solution. Comparing to our WebDriver classic implementation (where we do not fail) this would be a regression for those who want to switch to WebDriver BiDi. It's unclear how many pages will be affected and we know two major ones already.

Should the usage of location.href for redirecting a load really end-up as a failed navigation? Also in case of x.com I can see that they now use <meta http-equiv="refresh" content="0; url = ... for the redirect.

@OrKoN
Copy link
Contributor

OrKoN commented Oct 24, 2024

I believe from the user perspective the current behavior could be considered a bug if the user specifies waiting for the load event in the navigate call because that condition is not met on the success response. The abort errors gives a clearer understanding that the current navigation was not completed and the client needs to consider additional events. Alternatively, we could specify how waiting conditions could be fulfilled tolerating an aborted navigation but then it's not clear what the command should return since it's dealing with multiple navigations. I think adding the navigation committed wait condition (as requested in other issues) would allow not throwing an abort error.

@whimboo
Copy link
Contributor

whimboo commented Oct 24, 2024

Just to add a reference to the issue Alex mentioned in his last comment about the navigation committed event: #788

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants