Open Bug 1693857 Opened 4 years ago Updated 1 year ago

Support "unbeforeunload" user prompts in Marionette

Categories

(Remote Protocol :: Marionette, enhancement, P2)

Firefox 85
Desktop
All
enhancement
Points:
5

Tracking

(Not tracked)

People

(Reporter: teilo+bugzilla, Unassigned)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [webdriver:m9])

it appears when firefox is started with the --marionette argument that alerts do not happen (seen in firefox 85 and 87 nightly (2021-02-19) 64bit windows and 85 on linux)

steps to reproduce

start a firefox normally

  • navigate to http://localhost:8080/
  • click "New View" in the left hand bar
  • enter some random alpha-numeric characters for the View Name
  • select "List View" option
  • click OK
  • enter some random text in the "Description field"
  • Click "New Item" in the left hand bar

Observe a alert saying "This page is asking you to confirm that you want to leave - data you have entered may not be saved."

close firefox.

start firefox as if geckodriver had started it e.g.

  • firefox.exe "--marionette" "-foreground" "-no-remote" "-profile" "C:\\Users\\jnord\\AppData\\Local\\Temp\\rust_mozprofileN6VBXM"
  • navigate to http://localhost:8080/
  • click "New View" in the left hand bar
  • enter some random alpha-numeric characters for the View Name
  • select "List View" option
  • click OK
  • enter some random text in the "Description field"
  • Click "New Item" in the left hand bar

Observe the lack of any alert

This was extracted from observing the failure of https://github.com/jenkinsci/acceptance-test-harness/blob/master/src/test/java/core/FormValidationTest.java failing to show any alert window and the waitFor(Alert) timing out.

Just to be sure with alerts you only reference beforeunload prompts? Any other user prompt (alert, confirm, prompt) is still working fine?

Flags: needinfo?(teilo+bugzilla)

I think Jenkins only uses a couple of alert types and not sure if any of the others have selenium tests.
I'm more of a backend Dev and wouldn't have known this was how it's implemented until you asked the question, but I guess
https://github.com/jenkinsci/jenkins/blob/jenkins-2.280/core/src/main/resources/lib/form/confirm.js#L119 is where this happens.
I'll go poking to see if the other alert types we have work.
https://github.com/jenkinsci/jenkins/blob/jenkins-2.280/core/src/main/resources/lib/layout/confirmationLink.jelly#L48

Yeah, so this is a beforeunload prompt, and as given by the WebDriver spec those are implicitly dismissed:
https://w3c.github.io/webdriver/#dfn-execute-a-function-body#x16-user-prompts

User prompts that are spawned from beforeunload event handlers, are dismissed implicitly upon navigation or close window, regardless of the defined user prompt handler.

Note that right now we set a preference to turn those off globally, and as such you don't see these alerts even with navigations not triggered via Marionette itself.

Note that there is also https://github.com/w3c/webdriver/issues/1294 to get a better definition for handling beforeunload events. But as long as that hasn't been defined we will keep the code that we currently have.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(teilo+bugzilla)
Resolution: --- → WONTFIX

Hi Henrik,

Thanks for investigating and the link to the spec. However I disagree with your conclusion with the understanding of the spec

User prompts that are spawned from beforeunload event handlers, are dismissed implicitly upon navigation or close window, regardless of the defined user prompt handler.

Now - the navigation happens before there is a user prompt. that is the beforeunloaded prompt is not already showing then the navigation is attempted, but as a result of the navigation.

thus it is incorrect to dismiss something that does not exist at the time of the navigation

timeline

  1. page is rendered
  2. a navigation event occurs (the click)
  3. the beforeunload should fire and a prompt should display

what the spec is saying as I read it is that if you then did another naviagion (webdriver.navigate()) then the alter would be silently dismissed.

in other words

  1. page is rendered
  2. a navigation event occurs (the click)
  3. the beforeunload should fire and a prompt should display
  4. another navigation occurs (via webdriver.navigate) the alter gets dismissed unconditionally.

I do not see why you think your interpretation is the correct one, especially in something designed to test web interations, this would basically make testing beforeunloaded prompts essentially untestable.

Note that there is also https://github.com/w3c/webdriver/issues/1294 to get a better definition for handling beforeunload events

Firefox did not used to behave like this so if you feel the spec is ambiguous I would recommend that you revert to your prior handling.

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---

Well, beforeunload prompts are always opened when a navigation is about to start. And that is what the WebDriver spec covers here. It shouldn't be about user prompts that were opened before. And that's the same with the click + navigation here.

James, maybe you can weight in here and clarify the spec situation?

Flags: needinfo?(james)

I think it's correct that beforeunload prompts are untestable with current WebDriver.

Every beforeunload prompt is generated either by a navigation or closing a borwsing context. And per the spec any such prompt is instantly dismissed when a WebDriver session is active.

I agree that there are a number of problems here; certain scenarios are untestable, and the spec isn't very clearly written, but I believe that the current marionette implementation is consistent with what's intended by the current spec.

Flags: needinfo?(james)

FTR I filed https://github.com/w3c/webdriver/pull/1573, seems like PRs are more active than the GH issues.

Every beforeunload prompt is generated either by a navigation or closing a borwsing context

then why would the spec need to say the following:

User prompts that are spawned from beforeunload event handlers, are dismissed implicitly upon navigation or close window, regardless of the defined user prompt handler.

surely it would just say

User prompts that are spawned from beforeunload event handlers, are dismissed implicitly

given it is calling something out - and those callings are (as you say) the only way you can get to this situation it would seem to be superflous or alluding to something else (which would be more aligned with my understanding of it being a subsequent navigation)

A different alternative is that the href has got messed up and no one noticed.
ie the ref from navigation (upon navigation) should not be to #dfn-navigation but rather #navigation (which is section 10) and so we are talking about explicit navigation rather than implicit.

I understand now we are discussing interpretations of a spec that would be best taken to the spec itself, however the Mozilla interpretation does seem different (now) to other implementers, and would not be in the sprit of the spec.

See the updated GH pull request. The spec had an incorrect change applied when "Go" was renamed to "Navigate To" and then subsequently missing defintions was removed causing it to link to the generic navigate.

Thus firefoxs behaviour is incorrect. the prompts should only be dismissed on close or "navigate to" (aka explicit navigation not implicit).

Slightly updating the bug summary so that it reflects about which topic this bug is.

Status: REOPENED → NEW
Summary: alerts do not get fired → No "unbeforeunload" user prompts are getting opened

James, until we have a decision for that issue does setting dom.disable_beforeunload to false via the capabilities work for you?

Severity: -- → S3
Flags: needinfo?(teilo+bugzilla)
Priority: -- → P3
Flags: needinfo?(teilo+bugzilla)
Product: Testing → Remote Protocol

To get support for beforeunload user prompts we need to stop modifying the value of the dom.disable_beforeunload preference. Once done we should make sure that such a prompt is always dismissed when a navigation is started or a window is closed. At least this is the current definition of the spec.

But there is also https://github.com/w3c/webdriver/issues/1294 which requests a better handling of the user prompt especially when certain tests want to specifically test the different behavior of the page depending on if the user is accepting or dismissing the dialog.

James, not sure if we want to get this fixed for classic or need to take a special look for BiDi only. What do you think?

Type: defect → enhancement
Flags: needinfo?(james)
Summary: No "unbeforeunload" user prompts are getting opened → Support "unbeforeunload" user prompts in Marionette

I think all prompt behaviour is much easier in BiDi because we can send an event and have the client handle it, rather than having to install a specific user prompt handler. If there's still demand to change classic after BiDi is fixed maybe we should look at the options.

Flags: needinfo?(james)

Actually thinking more about it we need to get it fixed for Marionette. If we keep dom.disable_beforeunload turned off there is no way for BiDi to handle such kind of prompts. And if we turn it on we can get WebDriver Bidi working but would regress our WebDriver classic implementation.

So maybe we cover this bug in M7 and do the WebDriver BiDi implementation in M8. Lets discuss tomorrow.

Points: --- → 5
Priority: P3 → P1
Whiteboard: [webdriver:m7]

Note that a WebDriver classic spec decision is needed first. See https://github.com/w3c/webdriver/issues/1294 for the recent status.

Whiteboard: [webdriver:m7] → [webdriver:m7:blocked]
Priority: P1 → P2

It's too late for the milestone 7 and the next milestone has been pretty much filled-up. As such lets schedule for milestone 9.

Whiteboard: [webdriver:m7:blocked] → [webdriver:m9:blocked]
Whiteboard: [webdriver:m9:blocked] → [webdriver:m9]
You need to log in before you can comment on or make changes to this bug.