Implement save as feature for QTtestBrowser.
Created attachment 95752 [details] proposed patch
Comment on attachment 95752 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=95752&action=review > Tools/QtTestBrowser/launcherwindow.cpp:53 > , m_zoomAnimation(0) > + , m_reply(0) > { Please update this part of the patch, because it conflicts with ToT. > Tools/QtTestBrowser/launcherwindow.cpp:940 > + connect(m_reply, SIGNAL(finished()), this, SLOT(slotFinished())); I can't find slotFinished anywhere, but fileDownloadFinished. > Tools/QtTestBrowser/launcherwindow.cpp:950 > + if (fileName.isEmpty() || !m_reply->errorString().isNull()) > + QMessageBox::warning(this, QString("Download"), QString("Download failed.")); It fails for me, m_reply->errorString() returns always with "Unknown error."
Created attachment 95867 [details] proposed patch
Comment on attachment 95867 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=95867&action=review > Tools/QtTestBrowser/launcherwindow.cpp:991 > + if (fileName.isEmpty() && m_reply->error() != QNetworkReply::NoError) AND does not seem to be correct here, you need OR.
(In reply to comment #4) > (From update of attachment 95867 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95867&action=review > > > Tools/QtTestBrowser/launcherwindow.cpp:991 > > + if (fileName.isEmpty() && m_reply->error() != QNetworkReply::NoError) > > AND does not seem to be correct here, you need OR. Most probably if you have fileName.isEmpty() then you sould simply return, since the user pressed "Cancel" on the dialog, and only show an error message when m_reply->error() indicates an error.
Comment on attachment 95867 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=95867&action=review Later this could be extended with a simple progress bar, but I think this is not the scope of this patch. Because of the problems previously pointed out, I would say r- for this patch if I were a reviewer. > Tools/QtTestBrowser/launcherwindow.cpp:992 > + QMessageBox::warning(this, QString("Download"), QString("Download failed.")); Further more, the message shown here could be more expressive if you would display m_reply->errorString(). And I would use critical here, but that's only a nitpick.
Comment on attachment 95867 [details] proposed patch I agree with Andras, user shouldn't get "Download failed" error if he or she pressed cancel button. And crital message would be better than warning message. r- now, but I'll set r+ if you fix these things. > Tools/QtTestBrowser/launcherwindow.cpp:991 > + if (fileName.isEmpty() && m_reply->error() != QNetworkReply::NoError) I mean this would be good: if (fileName.isEmpty()) return; if (m_reply->error() != QNetworkReply::NoError) QMessageBox::critical(this, QString("Download"), QString("Download failed."));
Created attachment 96077 [details] proposed patch
Comment on attachment 96077 [details] proposed patch LGTM, r=me
Comment on attachment 96077 [details] proposed patch Clearing flags on attachment: 96077 Committed r88161: <http://trac.webkit.org/changeset/88161>
All reviewed patches have been landed. Closing bug.
Unreviewed --minimal buildfix landed in http://trac.webkit.org/changeset/88167
Revision r88161 cherry-picked into qtwebkit-2.2 with commit 1be60bc <http://gitorious.org/webkit/qtwebkit/commit/1be60bc> Revision r88167 cherry-picked into qtwebkit-2.2 with commit 0558b11 <http://gitorious.org/webkit/qtwebkit/commit/0558b11>