As discussed in the API review session in Szeged, there are a few simple API changes we should do before the release: * Try to change the type of the action property to the real enum * Removing originatingUrl property (no clear use-case known. miight be useful, but we should add it if we have a real use-case) * Rename modifiers to keyboardModifiers * Rename button to mouseButton
Created attachment 129096 [details] Patch
Comment on attachment 129096 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129096&action=review > Source/WebKit2/ChangeLog:7 > + [Qt] API changes to QWebNavigationRequest > + https://bugs.webkit.org/show_bug.cgi?id=78821 > + > + Reviewed by NOBODY (OOPS!). > + A summarized info about what changes are being made would be nice
Created attachment 129236 [details] Patch
Comment on attachment 129236 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129236&action=review > Source/WebKit2/UIProcess/API/qt/qwebnavigationrequest.cpp:29 > + QWebNavigationRequestPrivate(const QUrl& url, Qt::MouseButton mouseButton, > + Qt::KeyboardModifiers keyboardModifiers, QQuickWebView::NavigationType navigationType) I beliuve that the coding style tell to not split up these up into two lines, but keep them as one > Source/WebKit2/UIProcess/API/qt/qwebnavigationrequest.cpp:50 > +QWebNavigationRequest::QWebNavigationRequest(const QUrl& url, Qt::MouseButton mouseButton, > + Qt::KeyboardModifiers keyboardModifiers, QQuickWebView::NavigationType navigationType, QObject* parent) Same here > Source/WebKit2/UIProcess/API/qt/qwebnavigationrequest_p.h:41 > + QWebNavigationRequest(const QUrl& url, Qt::MouseButton mouseButton, Qt::KeyboardModifiers keyboardModifiers, > QQuickWebView::NavigationType navigationType, QObject* parent = 0); and here > Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:45 > +void QtWebPagePolicyClient::decidePolicyForNavigationAction(const QUrl& url, Qt::MouseButton mouseButton, > Qt::KeyboardModifiers keyboardModifiers, QQuickWebView::NavigationType navigationType, WKFramePolicyListenerRef listener) And here
Committed r109106: <http://trac.webkit.org/changeset/109106>
(In reply to comment #5) > Committed r109106: <http://trac.webkit.org/changeset/109106> Reopen, because 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&, Qt::MouseButton, Qt::KeyboardModifiers, QQuickWebView::NavigationType, const OpaqueWKFramePolicyListener*)’: /ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:55: error: case label value exceeds maximum value for type
I reverted the part that changes action to be an enum type in bug 79800 because was making our bot GCC unhappy when switching one type with labels from the other. The 'int' solution works and is OK since we are taking values from 2 different enums (one in WebView other in Experimental). It might be the price to pay for having the Experimental enums, but I wonder if there are other approaches we could take. Simon, Kenneth: do you have other ideas?
For reference: https://bugs.webkit.org/show_bug.cgi?id=73818 moved the DownloadRequest to a separate enum in experimental and http://trac.webkit.org/changeset/103331 fixes the build afterwards in a "similar" way we did here.
Patch for bug 80164 changes the 'action' property to the proper enum.