page succeeds only when all loadFinished signals succeed #328

wants to merge 2 commits into


None yet
3 participants

Every time a page loads an iframe through a callback (such as setTimeout), QWebPage will emit a loadFinished signal. I think that m_pageSuccess should indicate whether all loadFinished signals were successful, rather than simply indicating the final signal (previous behavior was to overwrite m_pageSuccess on each signal). This allows us to reliably detect iframe failures in capybara/cucumber tests and avoids intermittent failures.

@halogenandtoast halogenandtoast and 1 other commented on an outdated diff May 4, 2012

@@ -1515,4 +1515,50 @@ def which_for(character)
subject.source.should == "Hello\0World"
+ 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"
+ puts 'not-found'

halogenandtoast May 4, 2012


Left a puts in here


joshuanapoli May 4, 2012

thanks! I removed the puts in commit 9cbf7d2

@halogenandtoast halogenandtoast commented on the diff May 4, 2012

+ function goodFrame(){var divTag = document.createElement("div");divTag.innerHTML = "<iframe src='/success'></iframe>";document.body.appendChild(divTag);};
+ // emits WebPage::loadFinished(false) when called from timer
+ function badFrame(){var divTag = document.createElement("div");divTag.innerHTML = "<iframe src='/not-found'></iframe>";document.body.appendChild(divTag);};
+ function badThenGood() { badFrame(); setTimeout('goodFrame()',100); };
+ </script>
+ </head>
+ <body onload="setTimeout('badThenGood()',100)">
+ </body>
+ </html>
+ [200,
+ { 'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s },
+ [body]]
+ else

halogenandtoast May 4, 2012


Is this else needed?


samwgoldman May 4, 2012

Yeah, it would be nice if that before filter didn't get executed for every context in this spec file.


halogenandtoast commented May 4, 2012

Just a couple of small comments.

Thanks for taking a look at my pull request!

I'm closing this pull request, because it was incorporated by #311

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