Bug 38840

Summary: [Qt] The QWebPage crashes on history.pushState().
Product: WebKit Reporter: Jędrzej Nowacki <jedrzej.nowacki>
Component: HistoryAssignee: Jędrzej Nowacki <jedrzej.nowacki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fishd, jedrzej.nowacki, kenneth, tonikitoo
Priority: P2 Keywords: HTML5, Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 36464    
Attachments:
Description Flags
Fix v1
none
Fix v1
none
Fix v2
none
Fix v3 none

Jędrzej Nowacki
Reported 2010-05-10 04:07:11 PDT
This code crashes: QWebPage page; page.mainFrame()->setHtml("<html><body></body></html>"); page.mainFrame()->evaluateJavaScript("history.pushState()"); The code crashes inside the HistoryController::pushState because a null pointer (m_previousItem is not set). The problem is casued by a feature of the QWebFrame::setHtml(); it doesn't set HistoryItem. The easiest workaround is to use QWebFrame::load instead.
Attachments
Fix v1 (2.34 KB, patch)
2010-05-11 03:26 PDT, Jędrzej Nowacki 		no flags
DetailsFormatted DiffDiff
Fix v1 (3.83 KB, patch)
2010-05-11 07:54 PDT, Jędrzej Nowacki 		no flags
DetailsFormatted DiffDiff
Fix v2 (4.50 KB, patch)
2010-05-12 00:51 PDT, Jędrzej Nowacki 		no flags
DetailsFormatted DiffDiff
Fix v3 (4.16 KB, patch)
2010-05-18 02:01 PDT, Jędrzej Nowacki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jędrzej Nowacki
Comment 1 2010-05-11 03:26:41 PDT
Created attachment 55685 [details] Fix v1 This is a crash fix. I think it is the only thing we can do about it. Some history feature won't work.
Jędrzej Nowacki
Comment 2 2010-05-11 07:54:37 PDT
Created attachment 55702 [details] Fix v1 :-)
Jędrzej Nowacki
Comment 3 2010-05-11 09:48:30 PDT
Comment on attachment 55702 [details] Fix v1 It breaks layout tests... clearing flags
Jędrzej Nowacki
Comment 4 2010-05-12 00:51:54 PDT
Created attachment 55816 [details] Fix v2
Antonio Gomes
Comment 5 2010-05-15 10:07:09 PDT
It would be good to mention in the ChangeLog why this is needed, as you said in the comment #0: "The problem is casued by a feature of the QWebFrame::setHtml(); it doesn't set HistoryItem." ... and that it is based of this: /* ... \note This method will not affect session or global history for the frame. */ void QWebFrame::setHtml(const QString &html, const QUrl &baseUrl)
Antonio Gomes
Comment 6 2010-05-15 10:09:38 PDT
Darin Fisher might be a potention reviewer
Jędrzej Nowacki
Comment 7 2010-05-18 02:01:06 PDT
Created attachment 56338 [details] Fix v3 Changelog changes :-)
Kenneth Rohde Christiansen
Comment 8 2010-05-18 06:38:32 PDT
Yes, I remember that specific behaviour, nice that we have a test for it now.
WebKit Commit Bot
Comment 9 2010-05-19 21:18:11 PDT
Comment on attachment 56338 [details] Fix v3 Clearing flags on attachment: 56338 Committed r59815: <http://trac.webkit.org/changeset/59815>
WebKit Commit Bot
Comment 10 2010-05-19 21:18:18 PDT
All reviewed patches have been landed. Closing bug.
Darin Fisher (:fishd, Google)
Comment 11 2010-05-19 21:30:26 PDT
Comment on attachment 56338 [details] Fix v3 WebCore/loader/HistoryController.cpp:647 + if (!m_previousItem) this seems wrong to me. you probably meant to check m_currentItem here. note that m_currentItem is assigned to m_previousItem after the call to createTreeItem. this change means that the first page in a window cannot call pushState, which is not good.
Jędrzej Nowacki
Comment 12 2010-05-20 04:27:17 PDT
(In reply to comment #11) > (From update of attachment 56338 [details]) > WebCore/loader/HistoryController.cpp:647 > + if (!m_previousItem) > this seems wrong to me. you probably meant to check m_currentItem here. > note that m_currentItem is assigned to m_previousItem after the call to > createTreeItem. this change means that the first page in a window cannot > call pushState, which is not good. You are right. I created a bug for it (bug 39418)
Note You need to log in before you can comment on or make changes to this bug.