http/tests/navigation/go-back-to-error-page.html introduced in r75966, and fails because of missing fix for LoadHTMLStringItem::invoke() of Qt DRT: http://build.webkit.org/results/Qt%20Linux%20Release/r75966%20(26617)/http/tests/navigation/go-back-to-error-page-pretty-diff.html
I added http/tests/navigation/go-back-to-error-page.html to the Skipped list until fix: http://trac.webkit.org/changeset/76003
Created attachment 79783 [details] Patch
ping!
Comment on attachment 79783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79783&action=review I am not a reviewer, and you should probably get somebody a little more familiar with the Qt platform to review it, but this overall looks good to me! > Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:889 > +// API Candidate? What does this comment mean? API in what sense? (Is there a separate API for DRT?) > Tools/DumpRenderTree/qt/WorkQueueItemQt.cpp:31 > +#include "../../../Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.h" I am not familiar with the Qt port style, but what happens if this file gets moved into some sub-directory, etc? Do all such paths have to be updated? Is there no way to set it up so that you can just include "DumpRenderTreeSupportQt.h" and add this path to the include paths? > Tools/DumpRenderTree/qt/WorkQueueItemQt.h:85 > + LoadAlternateHTMLStringItem(const QString& content, const QString &baseURL, const QString &failingURL, QWebPage *page) Extra space here between "&baseURL," and "const QString"
Comment on attachment 79783 [details] Patch You say nothing about what is the problem in ChangeLog. Please fix that before landing. Also it is not so easy to review without knowing the problem, what caused it, and how you want to fix. Does it all have to do with ErrorPageExtension in QWebPage? This is how error pages are to be handled ..
Created attachment 87176 [details] Patch
(In reply to comment #4) > > What does this comment mean? API in what sense? (Is there a separate API for DRT?) Yes, DRTSupportQt just provides a private API to DumpRenderTree. It avoids the need to add public Qt API in order to just pass tests. I think I've addressed your other comment's and Antonio's in the patch.
Comment on attachment 87176 [details] Patch Rejecting attachment 87176 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2 Last 500 characters of output: ded at 98 with fuzz 1 (offset 18 lines). patching file Tools/QtTestBrowser/QtTestBrowser.pro Hunk #1 FAILED at 26. 1 out of 1 hunk FAILED -- saving rejects to file Tools/QtTestBrowser/QtTestBrowser.pro.rej patching file Tools/QtTestBrowser/launcherwindow.h Hunk #1 FAILED at 61. 1 out of 1 hunk FAILED -- saving rejects to file Tools/QtTestBrowser/launcherwindow.h.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Antonio Gomes', u'--fo..." exit_code: 1 Full output: http://queues.webkit.org/results/8285955
Comment on attachment 87176 [details] Patch Clearing flags on attachment: 87176 Committed r82489: <http://trac.webkit.org/changeset/82489>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/82489 might have broken Qt Linux Release The following tests are not passing: platform/qt/plugins/qt-qwidget-plugin.html plugins/application-plugin-plugins-disabled.html
(In reply to comment #11) > http://trac.webkit.org/changeset/82489 might have broken Qt Linux Release > The following tests are not passing: > platform/qt/plugins/qt-qwidget-plugin.html > plugins/application-plugin-plugins-disabled.html No, it's http://trac.webkit.org/changeset/82240/trunk/Source/WebKit.pri that caused this.