Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

add error message detail for page load failure #311

Closed
wants to merge 8 commits into from
Closed

add error message detail for page load failure #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5831441
page load failure should not cause the next command to fail
Apr 4, 2012
142047c
add error message detail for page load failure
Apr 4, 2012
b7e3868
page succeeds only when all loadFinished events succeed
Apr 26, 2012
9cbf7d2
remove debug output
May 4, 2012
7b4a3ba
Merge branch 'master' of https://github.com/thoughtbot/capybara-webkit
May 30, 2012
0aac807
Merge branch 'multiple-loadFinished-with-timers'
May 30, 2012
000f7da
standardize whitespace on condition statements
May 31, 2012
000f327
better names for javascript functions that trigger the loadFinished s…
May 31, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 70 additions & 0 deletions spec/driver_spec.rb
View file
Open in desktop
Expand Up @@ -104,6 +104,35 @@
end
end

context "error iframe app" do
before(:all) do
@app = lambda do |env|
case env["PATH_INFO"]
when "/inner-not-found"
[404, {}, []]
when "/outer"
body = <<-HTML
<html>
<body>
<iframe src=\"/inner-not-found\"></iframe>
</body>
</html>
HTML
[200,
{ 'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s },
[body]]
else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this else needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else clause handles the block at line 6, before { subject.visit("/hello/world?success=true") }

body = "<html><body></body></html>"
return [200, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, [body]]
end
end
end

it "raises error whose message references the actual missing url" do
expect { subject.visit("/outer") }.to raise_error(Capybara::Driver::Webkit::WebkitInvalidResponseError, /inner-not-found/)
end
end

context "redirect app" do
before(:all) do
@app = lambda do |env|
Expand Down Expand Up @@ -1641,4 +1670,45 @@ def which_for(character)
subject.window_handles.should_not include(last_handle)
end
end

context "timers app" do
before(:all) do
@app = lambda do |env|
case env["PATH_INFO"]
when "/success"
[200, {'Content-Type' => 'text/html'}, ['<html><body></body></html>']]
when "/not-found"
[404, {}, []]
when "/outer"
body = <<-HTML
<html>
<head>
<script>
function emit_true_load_finished(){var divTag = document.createElement("div");divTag.innerHTML = "<iframe src='/success'></iframe>";document.body.appendChild(divTag);};
function emit_false_load_finished(){var divTag = document.createElement("div");divTag.innerHTML = "<iframe src='/not-found'></iframe>";document.body.appendChild(divTag);};
function emit_false_true_load_finished() { emit_false_load_finished(); setTimeout('emit_true_load_finished()',100); };
</script>
</head>
<body onload="setTimeout('emit_false_true_load_finished()',100)">
</body>
</html>
HTML
[200,
{ 'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s },
[body]]
else
body = "<html><body></body></html>"
return [200, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, [body]]
end
end
end

it "raises error for any loadFinished failure" do
expect do
subject.visit("/outer")
sleep 1
subject.find("//body")
end.to raise_error(Capybara::Driver::Webkit::WebkitInvalidResponseError)
end
end
end
3 changes: 2 additions & 1 deletion src/Connection.cpp
View file
Open in desktop
Expand Up @@ -41,7 +41,7 @@ void Connection::startCommand() {
}

void Connection::pendingLoadFinished(bool success) {
m_pageSuccess = success;
m_pageSuccess = m_pageSuccess && success;
if (m_commandWaiting)
startCommand();
}
Expand All @@ -53,6 +53,7 @@ void Connection::writePageLoadFailure() {
}

void Connection::finishCommand(Response *response) {
m_pageSuccess = true;
m_runningCommand->deleteLater();
writeResponse(response);
}
Expand Down
22 changes: 20 additions & 2 deletions src/WebPage.cpp
View file
Open in desktop
Expand Up @@ -141,6 +141,7 @@ bool WebPage::javaScriptPrompt(QWebFrame *frame, const QString &message, const Q

void WebPage::loadStarted() {
m_loading = true;
m_errorPageMessage = QString();
}

void WebPage::loadFinished(bool success) {
Expand All @@ -153,7 +154,11 @@ bool WebPage::isLoading() const {
}

QString WebPage::failureString() {
return QString("Unable to load URL: ") + currentFrame()->requestedUrl().toString();
QString message = QString("Unable to load URL: ") + currentFrame()->requestedUrl().toString();
if (m_errorPageMessage.isEmpty())
return message;
else
return message + ": " + m_errorPageMessage;
}

bool WebPage::render(const QString &fileName) {
Expand Down Expand Up @@ -191,12 +196,16 @@ QString WebPage::chooseFile(QWebFrame *parentFrame, const QString &suggestedFile
}

bool WebPage::extension(Extension extension, const ExtensionOption *option, ExtensionReturn *output) {
Q_UNUSED(option);
if (extension == ChooseMultipleFilesExtension) {
QStringList names = QStringList() << getLastAttachedFileName();
static_cast<ChooseMultipleFilesExtensionReturn*>(output)->fileNames = names;
return true;
}
else if (extension == QWebPage::ErrorPageExtension) {
ErrorPageExtensionOption *errorOption = (ErrorPageExtensionOption*) option;
m_errorPageMessage = "Because of error loading " + errorOption->url.toString() + ": " + errorOption->errorString;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message is combined with Unable to load URL: http://example.com/ with a colon; not sure if the formatting you have here makes sense for that.

How about this: drop the colon on line 161 and downcase the 'B' in "Because".

Unable to load URL http://example.com/ because of an error loading http://example.net/: could not find The Net.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I improved the message formatting as you suggested in commit 1b99d2d.

return false;
}
return false;
}

Expand Down Expand Up @@ -241,6 +250,15 @@ void WebPage::handleUnsupportedContent(QNetworkReply *reply) {
Q_UNUSED(handler);
}

bool WebPage::supportsExtension(Extension extension) const {
if (extension == ErrorPageExtension)
return true;
else if (extension == ChooseMultipleFilesExtension)
return true;
else
return false;
}

QWebPage *WebPage::createWindow(WebWindowType type) {
Q_UNUSED(type);
return m_manager->createPage(this);
Expand Down
2 changes: 2 additions & 0 deletions src/WebPage.h
View file
Open in desktop
Expand Up @@ -48,6 +48,7 @@ class WebPage : public QWebPage {
virtual bool javaScriptConfirm(QWebFrame *frame, const QString &message);
virtual bool javaScriptPrompt(QWebFrame *frame, const QString &message, const QString &defaultValue, QString *result);
virtual QString chooseFile(QWebFrame * parentFrame, const QString &suggestedFile);
virtual bool supportsExtension(Extension extension) const;

private:
QString m_capybaraJavascript;
Expand All @@ -59,6 +60,7 @@ class WebPage : public QWebPage {
int m_lastStatus;
QString m_pageHeaders;
QStringList m_consoleMessages;
QString m_errorPageMessage;
QString m_uuid;
WebPageManager *m_manager;
};
Expand Down