Bug 78821

Summary: [Qt] API changes to QWebNavigationRequest
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: WebKit APIAssignee: Caio Marcelo de Oliveira Filho <cmarcelo>
Status: RESOLVED FIXED    
Severity: Normal CC: laszlo.gombos, maheshk, menard, ossy, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 78867, 79800, 80164    
Bug Blocks: 74403    
Attachments:
Description Flags
Patch
none
Patch kenneth: review+

Simon Hausmann
Reported 2012-02-16 08:15:16 PST
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
Attachments
Patch (13.12 KB, patch)
2012-02-27 14:01 PST, Caio Marcelo de Oliveira Filho 		no flags
DetailsFormatted DiffDiff
Patch (13.34 KB, patch)
2012-02-28 05:58 PST, Caio Marcelo de Oliveira Filho 		kenneth: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Caio Marcelo de Oliveira Filho
Comment 1 2012-02-27 14:01:28 PST
Created attachment 129096 [details] Patch
Kenneth Rohde Christiansen
Comment 2 2012-02-28 04:51:01 PST
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
Caio Marcelo de Oliveira Filho
Comment 3 2012-02-28 05:58:26 PST
Created attachment 129236 [details] Patch
Kenneth Rohde Christiansen
Comment 4 2012-02-28 06:02:14 PST
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
Caio Marcelo de Oliveira Filho
Comment 5 2012-02-28 08:12:14 PST
Committed r109106: <http://trac.webkit.org/changeset/109106>
Csaba Osztrogonác
Comment 6 2012-02-28 08:31:02 PST
(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
Caio Marcelo de Oliveira Filho
Comment 7 2012-02-28 09:14:03 PST
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?
Caio Marcelo de Oliveira Filho
Comment 8 2012-02-28 09:20:04 PST
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.
Caio Marcelo de Oliveira Filho
Comment 9 2012-03-02 09:09:56 PST
Patch for bug 80164 changes the 'action' property to the proper enum.
Note You need to log in before you can comment on or make changes to this bug.