[Webkit-unassigned] [Bug 90603] [EFL] Support the permission function of the Web Notification.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90603





--- Comment #9 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-07-12 20:25:35 PST ---
(From update of attachment 151199)
View in context: https://bugs.webkit.org/attachment.cgi?id=151199&action=review

>>> Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:80
>>> +    PermissionsMap::iterator iter = m_cachedPermissions.find(context->url().host());
>> 
>> Is it intentional to use host()? For example, is it OK to ignore the port number?
>> It looks WebKit2/mac uses SecurityOrigin for that same purpose.
> 
> You are right.
> I think I need to change about that.
> Thank you. :)

You could take a look at how we are doing this for Qt (I am not sure the patch was ever landed but one exists in bugzilla).

> Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:115
> +#if ENABLE(LEGACY_NOTIFICATIONS)
> +    LegacyPermissionRequestMap::iterator legacyIter = m_pendingLegacyPermissionRequests.find(String(domain));

Why support legacy notifications at all? They should go away no?

> Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.h:41
> +#if ENABLE(LEGACY_NOTIFICATIONS)

I would really let EFL just not support legacy notifications. You are adding much code and it makes it even harder removing the feature later.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.