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

Element retrieval commands do not invoke the user prompt handler #1086

Closed
thejohnjansen opened this issue Sep 6, 2017 · 17 comments
Closed
Assignees
Labels
Milestone

Comments

@thejohnjansen
Copy link
Contributor

I was just looking through this issue: mozilla/geckodriver#920 and I got to wondering why it is that Find Element does not have a step to handle an unexpected dialog the way GetElementText (for example) does:

Handle any user prompts and return its value if it is an error.

I don't remember discussing this, so I'm not sure if it's an omission by choice or accident...

@andreastt
Copy link
Member

@shs96c

@andreastt
Copy link
Member

Can we get some feedback here, @AutomatedTester?

@andreastt andreastt changed the title FindElement does not account for unexpected modal dialogs Element retrieval commands do not invoke the user prompt handler Oct 23, 2017
@andreastt
Copy link
Member

I think this is a bug.

@tayrawr
Copy link

tayrawr commented Dec 4, 2017

Any confirmation on whether or not this might be a bug?

@andreastt andreastt modified the milestones: Level 1, Level 2 Sep 21, 2018
@whimboo
Copy link
Contributor

whimboo commented Sep 21, 2018

Please note that the "Get Active Element" command also checks for user prompts.

@AutomatedTester
Copy link
Contributor

This is by choice and pretty sure how the open source project works. Perhaps @jimevans will know from IEDriver development. The modal exceptions are there if we are going to be doing some form of interaction but not while we are doing find element.

@shs96c
Copy link
Contributor

shs96c commented Sep 26, 2018

So long as this doesn't block executing the command, it's fine, but I think it's probably worth reviewing each command to see whether or not we need to add this language as we're not consistent (for example Get Element Text does the modal dialog check, but isn't an interaction, whereas Element Click doesn't appear to do the check at all)

@jimevans
Copy link
Contributor

The IE driver will block on an alert for find element, as it uses JavaScript (Selenium’s “automation atoms”) to execute the find. Therefore, it does handle user prompts before executing find element, and will continue to do so, even if that behavior is in violation of the spec.

@whimboo
Copy link
Contributor

whimboo commented Sep 26, 2018

So long as this doesn't block executing the command, it's fine, but I think it's probably worth reviewing each command to see whether or not we need to add this language as we're not consistent

I think that is a good idea. While writing tests for user prompts I also noticed that for switch to window we need this check. Is that still relevant? As it looks like it was originally added because of https://www.w3.org/2015/10/25-webdriver-minutes.html#action28, and then refactored by 9bc8839 (Safari now allows to switch the window with a user prompt open). But I actually don't understand why it is necessary anymore. Do other browsers still prevent us from switching the window? @jimevans, how does IE behave?

@whimboo
Copy link
Contributor

whimboo commented Sep 26, 2018

Also note that for click() and clear() I have #1303 open.

@jimevans
Copy link
Contributor

@whimboo The IE implementation supports switching windows with a user prompt open.

@whimboo
Copy link
Contributor

whimboo commented Sep 27, 2018

Great. So those before-mentioned dependencies are no longer present. Can we update the summary so that the issue mentions something like "Re-evaluate which commands need to check for user prompts"? Or should I file a separate issue about that?

@shs96c shs96c changed the title Element retrieval commands do not invoke the user prompt handler Review commands to ensure that user prompt handler is called Oct 1, 2018
@shs96c
Copy link
Contributor

shs96c commented Oct 1, 2018

@whimboo I've updated the issue name.

@CalebRouleau
Copy link

One solution to this problem requires bidirectional communication which would enable the test case to define a handler for alerts. This would keep the test case in the user's chair. It is a generic solution that does not require adding "and check for any alert and if it happens then throw an exception" into every other step in the spec. And then the web application code is exercised just like a user would exercise it. The test case can even simulate a user by adding a sleep or trying to click away from the alert.

@whimboo
Copy link
Contributor

whimboo commented Oct 29, 2018

@CalebRouleau, I wonder if your last comment was probably more meant for issue #1153? Or I miss the connection to this specific issue.

The resolution of our discussion by Friday is: https://www.w3.org/2018/10/26-webdriver-irc#T13-40-21

13:40:21 [AutomatedTester] RESOLUTION: Go read the spec issue and follow up on that one

@AutomatedTester, or @shs96c I'm actually not sure who should do it because no action item was logged. Can you remember?

Given that at least Safari is using Javascript for the various "Find Element" implementations we want to have those checks?

Also as talked with @shs96c on Friday there is the possibility that browsers can switch from tab modal dialogs to full modal dialogs (like in Firefox with the prompts.tab_modal.enabled preference). In such a situation we cannot switch to a different window without closing the modal dialog first. But based on a later conversation with @andreastt on Friday this seems problematic because such modal dialogs should be running in chrome scope. Should we really allow to interact with them when being in content scope?

@andreastt
Copy link
Member

The specification is written in such a way that it is agnostic about
whether browsers us tab modal or global modal dialogues. It used
to be the case that Safari used global modal dialogues that wouldn’t
let you switch tab within one window, but I no longer believe this
is the case. Irregardless, the idea is that WebDriver will let you
switch tab if the browser let’s you switch tab.

It should never be alowed to do element interaction (clicking, send
keys) with any non-web content browser feature. This is why we have
explicit commands to interact with e.g.
window.prompt("Enter some text:").

@whimboo whimboo changed the title Review commands to ensure that user prompt handler is called Element retrieval commands do not invoke the user prompt handler Oct 29, 2018
@whimboo
Copy link
Contributor

whimboo commented Oct 29, 2018

I'm going to revert the summary of the issue given that we have only spoken about the element retrieval commands at TPAC. To be consistent between drivers (including the IE driver) lets add the user prompt handler checks to those commands.

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

No branches or pull requests

8 participants