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

Already on GitHub? Sign in to your account

Support to control how JavaScript dialogs are handled #341

Closed
wants to merge 14 commits into
from

Conversation

Projects
None yet
8 participants
Contributor

danivovich commented May 23, 2012

Adds support to set if confirms and prompts should be accepted or rejected, as well as support to provide the text that should be sent to a JavaScript prompt. Also collects the alert, confirm, and prompt messages so that they can be asserted on.

Keeps the default return values if none of the new methods are called, so it should not change any existing behavior (accept confirms, reject prompts)

@mike-burns mike-burns commented on an outdated diff May 31, 2012

lib/capybara/driver/webkit.rb
+ browser.accept_js_confirms
+ browser.accept_js_prompts
+ yield
+ restore_js_dialog_state(confirm_js, prompt_js)
+ end
+
+ def reject_js_dialogs(&block)
+ confirm_js = browser.confirm_js?
+ prompt_js = browser.prompt_js?
+ browser.reject_js_confirms
+ browser.reject_js_prompts
+ yield
+ restore_js_dialog_state(confirm_js, prompt_js)
+ end
+
+ def js_dialog_input=(value)
@mike-burns

mike-burns May 31, 2012

Owner

Is there a use for this outside of confirm_js_dialogs_with? Why is this public?

@mike-burns mike-burns commented on an outdated diff May 31, 2012

src/JavascriptDialogMessages.cpp
@@ -0,0 +1,17 @@
+#include "JavascriptDialogMessages.h"
+#include "WebPage.h"
+
+JavascriptDialogMessages::JavascriptDialogMessages(WebPage *page, QStringList &arguments, QObject *parent) : Command(page, arguments, parent) {}
+
+void JavascriptDialogMessages::start()
@mike-burns

mike-burns May 31, 2012

Owner

Instead of doing a case statement, how about commands JavascriptAlertMessages, JavascriptConfirmMessages, etc.

@mike-burns mike-burns commented on an outdated diff May 31, 2012

src/JavascriptDialogMessages.cpp
+#include "JavascriptDialogMessages.h"
+#include "WebPage.h"
+
+JavascriptDialogMessages::JavascriptDialogMessages(WebPage *page, QStringList &arguments, QObject *parent) : Command(page, arguments, parent) {}
+
+void JavascriptDialogMessages::start()
+{
+ QString option = arguments()[0];
+ if (option.compare("Alert") == 0)
+ emit finished(new Response(true, page()->alertMessages()));
+ else if (option.compare("Confirm") == 0)
+ emit finished(new Response(true, page()->confirmMessages()));
+ else if (option.compare("Prompt") == 0)
+ emit finished(new Response(true, page()->promptMessages()));
+ else
+ emit finished(new Response(true, QString("")));
@mike-burns

mike-burns May 31, 2012

Owner

What is this else used for?

@mike-burns mike-burns commented on an outdated diff May 31, 2012

@@ -92,6 +106,59 @@ capybara-webkit supports a few methods that are not part of the standard capybar
page.driver.cookies["alpha"]
=> "abc"
+**confirm_js_dialogs**: confirm any Javascript dialog that is triggered by the page's Javascript (confirms and prompts)
+
+ # In Javascript:
+ if (confirm("Ok?"))
+ console.log("Hi");
+ else
+ console.log("Bye");
+ # In Ruby:
+ page.driver.confirm_js_dialogs do
@mike-burns

mike-burns May 31, 2012

Owner

I'm confused: why does this take a block?

Owner

mike-burns commented Jun 16, 2012

Hi @danivovich , I am interested in this feature. Any thoughts on my questions above?

Contributor

danivovich commented Jun 16, 2012

Thanks for the comments. I've been using my branch and would like to make some changes based on your comments as well as get this up to date with master. I haven't had a chance to work on it recently but I hope to have some time tomorrow to push some new code to this pull request

+1 Looking for functionality like this in our app.

addbrick commented Jul 3, 2012

+1 ditto

+1 as well

Owner

mike-burns commented Jul 10, 2012

Hi @danivovich . Recently @jferris has done a bunch of refactoring around the library. Can you rebase, resolve conflicts, and then I'll pull this in?

Thanks,
-Mike

Contributor

danivovich commented Jul 10, 2012

@mike-burns, @jferris: I like the refactoring, conflicts were straight forward. The test suite is green for me, and my project that uses the branch also works as expected. This should be good to go now.

Owner

mike-burns commented Jul 10, 2012

Ran into a mess when I went to merge this (you had merged master into your branch, which does that), so I made another branch: js_dialogs_2.

However, something is up with my machine and master is currently not passing, so I've pushed that branch and hopefully someone else will come by and merge this.

Contributor

danivovich commented Jul 13, 2012

Is there anything I can do to help with this merging issue?

Owner

mike-burns commented Jul 13, 2012

This is on us now, @danivovich . My local capybara-webkit setup needs to be fixed (I think it's a Qt problem), but maybe @jferris or @halogenandtoast will pull this soon.

Thanks for the patience,
-Mike

Owner

jferris commented Jul 13, 2012

The errors @mike-burns was running into are pre-existing issues with Qt 4.8. I ran @danivovich's changes on Qt 4.7.4 and got a clean rake. This is now merged into master.

@jferris jferris closed this Jul 13, 2012

vrish88 commented Jul 30, 2012

@jferris Do you have an idea of when this'll be released in a versioned gem?

I see mention of the use of page.driver.alert_messages to access alert messages, but it doesn't look like it exists any longer. This would be nice, as well as commands to accept or deny alerts/confirmation popups.

Contributor

danivovich commented Sep 7, 2012

@redconfetti This was merged into master, but has not yet been part of a released version of the gem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment