Bug 80702

Started it yesterday (it is really crude yet), it is missing a) Finish the provider b) Update the notification permission manager https://gist.github.com/2006840

*** Bug 84484 has been marked as a duplicate of this bug. ***

Created attachment 147101 [details] Adding Desktop Notification permission type into Qt I decided to split the patch in 2: a) Having support for permission type Desktop Notification (this patch) and b) The notification plugin (should be next patch)

Ouch! Forgot the CLs. I will update a new patch soon.

Attachment 147101 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/UIProcess/API/qt/qwebpermis..." exit_code: 1 Source/WebKit2/UIProcess/API/qt/tests/qmltests/common/notification.html:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 147135 [details] Changelog, fixed coding style in HTML test file Fixed code style, added ChangeLog information.

Added new bug #88899 to reference the plugin task.

Could you please clear the review flag and obselete the patch that is not for review (I believe the first one is not).

Alexis Done, previous patch is marked as obsolete.

Comment on attachment 147135 [details] Changelog, fixed coding style in HTML test file View in context: https://bugs.webkit.org/attachment.cgi?id=147135&action=review > Source/WebKit2/UIProcess/API/qt/qwebpermissionrequest.cpp:33 > + , geoRequest(geo) > + , notifyRequest(notify) geolocationRequest notificationRequest > Source/WebKit2/UIProcess/API/qt/qwebpermissionrequest.cpp:72 > +QWebPermissionRequest::QWebPermissionRequest(WKSecurityOriginRef securityOrigin > + , WKGeolocationPermissionRequestRef geo > + , WKNotificationPermissionRequestRef notify > + , QWebPermissionRequest::RequestType type > + , QObject* parent) wrong indentation, this should just be one line Wouldn't two different ctors make sense? > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_notification.qml:20 > + //Must be false by default space > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_notification.qml:24 > + if (!permission.allow) { > + permission.allow = true > + } else > + console.log("Fail: permission must be set to false") inconsistent coding style

Kenneth Thanks a lot for your comments, I will upload a patch with fixes. I have just 1 question: about the indentation in QWebPermissionRequestPrivate constructor. Do you want all parameters to be in 1 single line? If I understood correctly, from the webkit coding sytle, it says: "Each member (and superclass) should be indented on a separate line, with the colon or comma preceding the member on that line." and has an example similar to the patch. Next, about the constructor: currently it has 4 parameters. I can see that if in future we need to treat more permission types, it would make sense to have multiple constructors. But ATM, there are only 2 permission types, so I feel that the code will be simpler (i.e. less member functions) in this way. Best regards Adenilson

Created attachment 147473 [details] Fixing issues with naming convention, indenting

Comment on attachment 147473 [details] Fixing issues with naming convention, indenting LGTM, and no one seemed to have looked at it in a month...

Comment on attachment 147473 [details] Fixing issues with naming convention, indenting Clearing flags on attachment: 147473 Committed r122478: <http://trac.webkit.org/changeset/122478>

All reviewed patches have been landed. Closing bug.