Summary: | [Qt] Extend QQuickWebview::navigationRequested API | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Hausmann <hausmann> | ||||||
Component: | WebKit API | Assignee: | Rafael Brandao <rafael.lobo> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cmarcelo, gopal.1.raghavan, jesus, laszlo.gombos, menard, ossy, vestbo, webkit.review.bot, zoltan | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 74403 | ||||||||
Attachments: |
|
Description
Simon Hausmann
2011-12-05 03:54:25 PST
Working on this, I've realized we get the WebProcess crashed once we try to accept a navigation request with Download (and this without my working patch). I will show this to Jesus tomorrow to see if this actually a bug or if I'm doing something wrong. Besides the frame requesting url, would we need anything else? So I could also add it when I submit it tomorrow. Created attachment 119239 [details]
Patch
Comment on attachment 119239 [details]
Patch
Is it possible to have some test?
(In reply to comment #3) > (From update of attachment 119239 [details]) > Is it possible to have some test? Sure. I didn't put this right away because it was not working before my patch, I confirmed with Jesus today that it is a bug. As there was no qml test for it before, we just missed that regression. Do you think I should add a test that fails here so it could be fixed in another patch? The test is gonna be added in a separate patch: https://bugs.webkit.org/show_bug.cgi?id=74541 When it lands, I will just change it here in this one to reflect the change in the enum. Created attachment 119478 [details]
Patch
Comment on attachment 119478 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119478&action=review Looks good in general, but there's a small memory leak that should be fixed. > Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:105 > + WKURLRef frameURL = WKFrameCopyURL(frame); > WKURLRef requestURL = WKURLRequestCopyURL(request); Memory leak, the url refs need to be kept in RetainPtrs and adopted: WKRetainPtr<WKURLRef> frameURL(AdoptWK, WKFrameCopyURL(frame)); Comment on attachment 119478 [details]
Patch
I'd like to get this change in, so r=me and I'll fix the leak when landing (trivial :)
Committed r103330: <http://trac.webkit.org/changeset/103330> It broke the build: cc1plus: warnings being treated as errors /ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp: In member function ‘void QtWebPagePolicyClient::decidePolicyForNavigationAction(const QUrl&, const QUrl&, Qt::MouseButton, Qt::KeyboardModifiers, const OpaqueWKFramePolicyListener*)’: /ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:56: error: case label value exceeds maximum value for type (In reply to comment #10) > It broke the build: > > cc1plus: warnings being treated as errors > /ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp: In member function ‘void QtWebPagePolicyClient::decidePolicyForNavigationAction(const QUrl&, const QUrl&, Qt::MouseButton, Qt::KeyboardModifiers, const OpaqueWKFramePolicyListener*)’: > /ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:56: error: case label value exceeds maximum value for type Fixed with http://trac.webkit.org/changeset/103331 |