Bug 107438

Summary: [Qt][WK1] Permission request callbacks for non-legacy notifications
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: WebKit QtAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: jturcotte
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 106699    
Bug Blocks:    
Attachments:
Description Flags
Patch
jturcotte: review+
Patch none

Allan Sandfeld Jensen
Reported 2013-01-21 03:50:58 PST
QtWebKit(1) is missing an implementation tracking and calling the permission request callbacks when using Web Nofications in non-legacy mode.
Attachments
Patch (6.01 KB, patch)
2013-01-21 03:56 PST, Allan Sandfeld Jensen 		jturcotte: review+
DetailsFormatted DiffDiff
Patch (5.79 KB, patch)
2013-01-21 06:02 PST, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Allan Sandfeld Jensen
Comment 1 2013-01-21 03:56:37 PST
Created attachment 183756 [details] Patch
Jocelyn Turcotte
Comment 2 2013-01-21 05:37:20 PST
Comment on attachment 183756 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183756&action=review > Source/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp:332 > +#if ENABLE(NOTIFICATIONS) > +void NotificationPresenterClientQt::requestPermission(ScriptExecutionContext* context, PassRefPtr<NotificationPermissionCallback> callback) > +{ This is pretty much duplicated code, could you factor out the common code of the two implementations in a separate method?
Allan Sandfeld Jensen
Comment 3 2013-01-21 06:02:15 PST
Created attachment 183768 [details] Patch
Jocelyn Turcotte
Comment 4 2013-01-21 06:18:59 PST
Comment on attachment 183756 [details] Patch Humm, it doesn't look as good as I thought, so we might as well duplicate the code. - NotificationPermissionCallback is defined behind ENABLE(NOTIFICATIONS) - Adding to a different list depending on the callback type isn't great r+ing the first one
Allan Sandfeld Jensen
Comment 5 2013-01-21 06:31:54 PST
Committed r140330: <http://trac.webkit.org/changeset/140330>
Note You need to log in before you can comment on or make changes to this bug.