This feature is required to handle some custom navigation policies in TouchWebView.
Created attachment 110046 [details] implementation of navigation policy Implemented navigationPolicyForUrl. This is similar to DesktopWebView implementation. It provides the flexibiltiy to specify custom navigation policies for TouchWebView. Regarding testcase. Looks like qmltests are disabled for qt5. I also need some suggestion on handling mouseClick's. Thanks,
Comment on attachment 110046 [details] implementation of navigation policy View in context: https://bugs.webkit.org/attachment.cgi?id=110046&action=review > Source/WebKit2/UIProcess/API/qt/qtouchwebview.h:47 > + enum NavigationPolicy { > + UsePolicy, > + DownloadPolicy, > + IgnorePolicy > + }; > + I am a bit confused why we have to define a new set of enums here and do conversions to the enums defined in the base class.
The PolicyAction enums defined in PolicyInterface.h is not available to qml. The current implementation enables the following in qml: TouchWebView { id: webView ...... ..... function navigationPolicyForUrl(url, button, modifiers) { // link delegation if (button == Qt.LeftButton && pageLoaded) { return webView.TouchWebView.IgnorePolicy } return webView.TouchWebView.UsePolicy } }
Comment on attachment 110046 [details] implementation of navigation policy View in context: https://bugs.webkit.org/attachment.cgi?id=110046&action=review > Source/WebKit2/UIProcess/qt/qtouchwebpageproxy.h:43 > + QTouchWebPageProxy(QTouchWebViewPrivate*, TouchViewInterface*, ViewportInteractionEngine*); Why do you need QTouchWebViewPrivate class here? Why PolicyInterface is not good? Also, I would change order of constructor parameters to make it the same as in QtWebPageProxy.
Created attachment 110739 [details] updated patch with reviewer comments Good point. Fixed the interface. Thanks
I am still not convinced that we have to manipulate the lower level C++ code which throws away the class inheritance just to satisfy a drawback in the higher level qml binding. What prevents PolicyInterface.h being available to qml? (In reply to comment #3) > The PolicyAction enums defined in PolicyInterface.h is not available to qml. The current implementation enables the following in qml: > > TouchWebView { > id: webView > ...... > ..... > function navigationPolicyForUrl(url, button, modifiers) { > // link delegation > if (button == Qt.LeftButton && pageLoaded) { > return webView.TouchWebView.IgnorePolicy > } > return webView.TouchWebView.UsePolicy > } > }
Ideally, if QTouchWebView could be derived out of PolicyInterface we should be all set. Although, PolicyInterface looks like an api it is not publicly available within UIProcess/API/qt. Therefore QTouchWebViewPrivate inherits it and not QTouchWebView. Because of this restriction, we need the translation of enums using toPolicyAction. Hope this clarifies the situation. Thanks
(In reply to comment #7) > Ideally, if QTouchWebView could be derived out of PolicyInterface we should be all set. Although, PolicyInterface looks like an api it is not publicly available within UIProcess/API/qt. Therefore QTouchWebViewPrivate inherits it and not QTouchWebView. Because of this restriction, we need the translation of enums using toPolicyAction. > > Hope this clarifies the situation. > Thanks Thanks. It's pretty clear now the problem is not qml itself but the hidden PolicyInterface. I think we should make it public. Any objections, Kenneth and Kling?
Comment on attachment 110739 [details] updated patch with reviewer comments View in context: https://bugs.webkit.org/attachment.cgi?id=110739&action=review > Source/WebKit2/UIProcess/API/qt/tests/tests.pro:3 > +lessThan(QT_MAJOR_VERSION, 6): SUBDIRS += qmltests Would this possibly break the buildbot? If not, we can simply remove the condition. Or it should be the other way around? I will add Ossy for commenting this.
I will update Qt5 on the bot tomorrow evening. Please wait a little bit with landing.
Created attachment 110864 [details] updated patch That change to tests.pro is unintentional. I didn't mean to include it in this patch. Here is an updated patch.
We probably should hold off the implementation until the proposed API change is made. https://lists.webkit.org/pipermail/webkit-qt/2011-October/001984.html
(In reply to comment #11) > Created an attachment (id=110864) [details] > updated patch > > That change to tests.pro is unintentional. I didn't mean to include it in this patch. > Here is an updated patch. Why not include this change (a modified version) otherwise the qmltests are not built nor run at all?
qmltests is a separate issue. Also, since this patch doesn't seem to be moving forward, I have attached a separate patch to enable qmltests here https://bugs.webkit.org/show_bug.cgi?id=70037
(In reply to comment #14) > qmltests is a separate issue. Also, since this patch doesn't seem to be moving forward, I have attached a separate patch to enable qmltests here https://bugs.webkit.org/show_bug.cgi?id=70037 Sounds good. While I was compiling the qmltests code, I had to "make install" webkit first. I am not sure if this will be an issue on the bot. Anyway, we can ask Ossy's help to review the patch.
Comment on attachment 110864 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=110864&action=review We want to do this in a different way > Source/WebKit2/ChangeLog:8 > + Implemented navigationPolicyForUrl. This is similar to DesktopWebView implementation. It provides the flexibiltiy to specify custom navigation policies for TouchWebView. Please break that line > Source/WebKit2/UIProcess/API/qt/tests/qmltests/TouchWebView/tst_navigationPolicyForUrl.qml:16 > + function navigationPolicyForUrl(url, button, modifiers) { > + if (button == Qt.MiddleButton && modifiers & Qt.ControlModifier) { > + otherWebView.page.load(url) > + return TouchWebView.IgnorePolicy > + } > + return TouchWebView.UsePolicy > + } We decided to do this differently. Simon had some better ideas. Please check the ML
(In reply to comment #9) > (From update of attachment 110739 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=110739&action=review > > > Source/WebKit2/UIProcess/API/qt/tests/tests.pro:3 > > +lessThan(QT_MAJOR_VERSION, 6): SUBDIRS += qmltests > > Would this possibly break the buildbot? If not, we can simply remove the condition. Or it should be the other way around? I will add Ossy for commenting this. I checked, it builds fine on the bot.
Should we close this now that all we have is a single QQuickWebView?! :)
=== Bulk closing of Qt bugs === If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary. If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.