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 browsingContext.navigate if navigation is aborted #799

Open
sadym-chromium opened this issue Oct 22, 2024 · 2 comments
Open

Fail browsingContext.navigate if navigation is aborted #799

sadym-chromium opened this issue Oct 22, 2024 · 2 comments
Labels
module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group

Comments

@sadym-chromium
Copy link
Contributor

sadym-chromium commented Oct 22, 2024

Currently, the browsingContext.navigate command's await a navigation succeeds in case of "navigation aborted".

Repro: https://github.com/web-platform-tests/wpt/pull/48751/files
Currently, there is a WPT test test_with_new_navigation_inside_page which is not aligned with the spec.

Spec:

Having that, either WPT test or spec should be changed. We propose the latest: change the spec, so that if the navigation is aborted, the command will fail:

If event received is "navigation failed" or "navigation aborted" return error with error code unknown error.

@sadym-chromium sadym-chromium added the needs-discussion Issues to be discussed by the working group label Oct 22, 2024
sadym-chromium added a commit to GoogleChromeLabs/chromium-bidi that referenced this issue Oct 22, 2024
@OrKoN OrKoN added the module-browsingContext Browsing Context module label Oct 23, 2024
@OrKoN
Copy link
Contributor

OrKoN commented Oct 24, 2024

Related #112

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Fail `browsingContext.navigate` if navigation is aborted.

The full IRC log of that discussion <AutomatedTester> topic: Fail `browsingContext.navigate` if navigation is aborted
<AutomatedTester> github: https://github.com//issues/799
<AutomatedTester> sadym: we have the issue where the user navigated to a page that had a script redirect which then aborted the navigate
<AutomatedTester> ... I looked at spec. The spec says we shouldn't fail
<jgraham> q+
<AutomatedTester> ... what is the expected behaviour if the navigation is aborted? Should we error or wait until redirection has completed?
<AutomatedTester> ack next
<AutomatedTester> jgraham: we're not talking about a redirect but a script that creates a new navigation
<jdescottes> q+
<AutomatedTester> ... it functionally looks like a redirect but it's doing a stop and then navigate
<AutomatedTester> ... I think we should tell the user that we have stopped the navigation and have started a new one
<orkon> q+
<AutomatedTester> .. I know this might look weird for the users
<AutomatedTester> sadym: I would expected <missed what was said>
<AutomatedTester> ack next
<AutomatedTester> jgraham: In the issue you have said that we should treat it like a failure
<orkon> q-
<AutomatedTester> ... it seems weird that we are failing when a new navigation has started
<AutomatedTester> jdescottes: I think we want to have a convenience way to handle this situation
<orkon> q+
<sadym> I would expect either the command to fail or to finish the navigation to "complete", where I can expect the page is ready
<AutomatedTester> ... I don't think it makes sense to error here
<sadym> q+
<orkon> q-
<AutomatedTester> ... I think the webdriver classic way handled this quite well
<AutomatedTester> ... and following that way will help people migrate easier
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> sadym: I think we are missing 2 concepts. DOM lifecycle and navigation
<AutomatedTester> ... if we do DOM then it makes sense
<AutomatedTester> ... but while we mix these concepts then its going to cause issues
<AutomatedTester> s/missing 2 concepts/mixing 2 concepts
<AutomatedTester> q?
<AutomatedTester> jgraham: I think I need to reread this issue and come back to you
<AutomatedTester> s/I would expected <missed what was said>/I would expect either the command to fail or to finish the navigation to "complete", where I can expect the page is ready
<sadym> * if we do navigation then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group
Projects
None yet
Development

No branches or pull requests

3 participants