WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
52614
[Qt] Fix LoadHTMLStringItem::invoke() after
r75966
https://bugs.webkit.org/show_bug.cgi?id=52614
Summary
[Qt] Fix LoadHTMLStringItem::invoke() after r75966
Csaba Osztrogonác
Reported
2011-01-18 00:40:43 PST
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
Attachments
Patch
(8.83 KB, patch)
2011-01-21 13:40 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(14.28 KB, patch)
2011-03-28 12:00 PDT
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
2011-01-18 00:44:46 PST
I added http/tests/navigation/go-back-to-error-page.html to the Skipped list until fix:
http://trac.webkit.org/changeset/76003
Robert Hogan
Comment 2
2011-01-21 13:40:22 PST
Created
attachment 79783
[details]
Patch
Robert Hogan
Comment 3
2011-03-27 05:49:39 PDT
ping!
Jessie Berlin
Comment 4
2011-03-27 08:12:06 PDT
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"
Antonio Gomes
Comment 5
2011-03-27 21:59:21 PDT
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 ..
Robert Hogan
Comment 6
2011-03-28 12:00:20 PDT
Created
attachment 87176
[details]
Patch
Robert Hogan
Comment 7
2011-03-28 12:02:18 PDT
(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.
WebKit Commit Bot
Comment 8
2011-03-30 13:17:31 PDT
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
WebKit Commit Bot
Comment 9
2011-03-30 13:23:34 PDT
Comment on
attachment 87176
[details]
Patch Clearing flags on attachment: 87176 Committed
r82489
: <
http://trac.webkit.org/changeset/82489
>
WebKit Commit Bot
Comment 10
2011-03-30 13:23:38 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 11
2011-03-30 14:41:19 PDT
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
Robert Hogan
Comment 12
2011-03-30 15:16:10 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug