Skip to content
This repository has been archived by the owner. It is now read-only.

When I use a Select tag with an onChange event that causes a prompt Selenium freezes. #3508

Closed
lukeis opened this issue Mar 3, 2016 · 11 comments
Closed

Comments

@lukeis
Copy link
Member

@lukeis lukeis commented Mar 3, 2016

Originally reported on Google Code with ID 3508

What steps will reproduce the problem?

On a page with a select control and an on change event which causes a prompt, when
I use selenium's Select object that causes the prompt, selenium freezes. 

I have tried this on Internet Explorer, and firefox. This worked as expected on Firefox.


What is the expected output? What do you see instead?
I should get control back, but it hangs. 

Selenium version: 2.20
OS: Win XP
Browser: Internet Explorer
Browser version:8.0.6001 


Please provide any additional information below. A sample reduced test
case, or a public URL that demonstrates the problem will intrigue our merry
band of Open Source developers far more than nothing at all: they'll be far
more likely to look at your problem if you make it easy for them!

The select that I use is: (this is a reproduction from something that I found in a
product)
         <select id='selection' onchange="if (this.value == '2') if (!confirm('2?'))
this.value = '1';">
         <option value="1"> 1 </option>
         <option value="2"> 2 </option>
      </select>


Here is a reduced test case that I use 
        @Test
    public void trySomething() { 
        driver.get("http://172.16.206.129/Something.html");
        try { 
            Thread.sleep(3000);
            WebElement elem = driver.findElement(By.id("selection"));
            Select sel = new Select(elem);
            sel.selectByIndex(1);
        } catch(Exception e) { 
        }
    } 

note: the url that I am using in this example is a my local machine.  

Reported by danderson.75 on 2012-03-07 01:38:19

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

This is a regrettable and suboptimal behavior. It is also entirely expected. I have
updated the wiki page[1] with a full explanation of why this is occurring in IE. Suggestions
and/or patches on how to overcome the issue without severely rearchitecting the driver
are welcome.

[1] http://code.google.com/p/selenium/wiki/InternetExplorerDriver

Reported by james.h.evans.jr on 2012-03-07 03:19:58

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Reported by barancev on 2012-05-11 05:42:05

  • Labels added: Component-WebDriver, Browser-IE, GettingInvolved

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Issue 4598 has been merged into this issue.

Reported by barancev on 2012-09-28 08:47:22

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Hi, James,

I am not familiar with IE Driver Arch enugh to judge whether my solution will work
or not or whether this means a severely re-architecture.

When WD hangs by alert() during some js execution (submit form, on close, etc), the
WD will keep executing IECommandExecutor::OnGetResponseLength, while the IE is waiting
for some action on the modal dialog poped. 

Currently, the Alert detaction logic lies in here: IECommandExecutor::DispatchCommand.
WD will check for alert before executing an command. 

I am thinking whether following refactoring will work:
  - refactor the alert detection logic into a seperate method, and make both IECommandExecutor::OnGetResponseLength
and IECommandExecutor::DispatchCommand call this logic.

Please let me know how do you think, James! Thanks very much!

Adam

Reported by adwu73@hotmail.com on 2012-11-10 13:26:48

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Reported by barancev on 2012-11-11 18:51:57

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

In point of fact, that won't work at all. Under the covers, when the Select class calls
WebElement.click() on the appropriate <option> element. For an <option> element in
IE, this is the equivalent of setting the .selected property of the element, and manually
firing the onchange event of the parent <select> element. It's this firing of the event
where the JavaScript execution (understandably and expectedly) "hangs".  Though the
temptation would be to do the JavaScript execution on a new thread, but with IE, it's
not that simple. You have to marshal the COM objects to the new thread.

This is also the reason that your solution with IECommandExecutor::OnGetResponseLength
won't work. The command executor's message loop will be blocked inside DispatchCommand,
and the subsequent SendMessage call from IESession::ExecuteCommand will hang waiting
for a return. You also can't change the SendMessage to a PostMessage, because you *need*
the return value from the WD_GET_RESPONSE_LENGTH message to tell you the command has
completed. 

Marshaling the objects onto another thread and performing the JavaScript execution
there may be your best bet. Again, if you can provide a patch that can fix this, I'll
be happy to review it.

Reported by james.h.evans.jr on 2012-11-12 10:41:50

  • Status changed: NeedsClarification

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Hi James, 

Thanks very much for your quick response. 

Please forgive me if I am repeating myself again, I just want to make sure that I have
explained my thoughts throughly. 

This is a log on my machine by running a demo that reproduce this issue with submitting
forms. As you explained the JS execution hangs on here waiting for the popup to be
manually dismissed.

T 2012-11-12 21:34:56:798 Script.cpp(576) Entering Script::CreateAnonymousFunction

However, I noticed that IECommandExecutor::OnGetResponseLength will run continuously
until I manually dismissed the alert, so that's way I am wondering whether we can put
the alert-dismiss logic here in the IECommandExecutor::OnGetResponseLength.

Anyway, I think following log will make my point very clear, so please let me know
whether you think this is a viable solution. If not, I will go back to learn the code
more and try to make this patch your way.

Regards!

Adam


T 2012-11-12 21:34:56:798 Script.cpp(40) Entering Script::Initialize
T 2012-11-12 21:34:56:798 Script.cpp(87) Entering Script::AddArgument(ElementHandle)
T 2012-11-12 21:34:56:798 Script.cpp(92) Entering Script::AddArgument(IHTMLElement*)
T 2012-11-12 21:34:56:798 Script.cpp(98) Entering Script::AddArgument(VARIANT)
T 2012-11-12 21:34:56:798 Script.cpp(210) Entering Script::Execute
T 2012-11-12 21:34:56:798 Script.cpp(576) Entering Script::CreateAnonymousFunction
T 2012-11-12 21:34:56:820 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:842 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:853 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:864 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:874 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:885 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:896 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:907 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:917 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:928 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength
T 2012-11-12 21:34:56:939 IECommandExecutor.cpp(189) Entering IECommandExecutor::OnGetResponseLength

Reported by adwu73@hotmail.com on 2012-11-12 13:50:23


- _Attachment: [iedriver-2012-11-12-13-34-53.log](https://storage.googleapis.com/google-code-attachments/selenium/issue-3508/comment-7/iedriver-2012-11-12-13-34-53.log)_

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Of course, you're right, the session object is on a different thread from the IECommandExecutor,
so the WD_GET_RESPONSE_LENGTH SendMessage() call wouldn't hang. That's what I get for
trying to go from memory, and reading source code on the web rather than tracing through
in the IDE.

Nevertheless, it's still a bad idea to short-circuit the call to OnGetResponseLength
the way you propose. As soon as you do that, you'll have an undefined value in the
response when the session sends the WD_GET_RESPONSE message. Even if you put a dummy
value in the response in the OnGetResponseLength, how do you guarantee it's the correct
response? Are you proposing to force that method to understand the semantics of every
command? That'd be a pretty big refactor. Also, though I confess I haven't thought
it all the way through, I'm not sure that approach wouldn't cause real issues with
the alert API.

If you have a patch using your approach that you think addresses the issue, I'll look
it over, but I still think marshaling the objects and performing the JavaScript execution
on a separate thread will be a more robust approach.

Reported by james.h.evans.jr on 2012-11-12 16:09:10

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

This issue was closed by revision r18129.

Reported by james.h.evans.jr on 2012-11-12 22:19:29

  • Status changed: Fixed

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Hi, James,

I noticed that you have fixed this, thanks very much, I will download the fix and test
with my application as well.

Regards!

Adam

Reported by adwu73@hotmail.com on 2012-11-12 23:32:53

@lukeis
Copy link
Member Author

@lukeis lukeis commented Mar 3, 2016

Reported by luke.semerau on 2015-09-17 18:14:55

  • Labels added: Restrict-AddIssueComment-Commit

@lukeis lukeis closed this Mar 3, 2016
@SeleniumHQ SeleniumHQ locked and limited conversation to collaborators Mar 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant