Bug 170014 – [GTK] Remove notifications guards from GTK API layer

• Component: WebKitGTK
• Status: RESOLVED FIXED

[ManDay]

Created attachment 305207 [details]
Error as produced by cmake ; make

Despite USE_LIBNOTIFY=OFF, the following error occurs during compilation of Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/gtk/WebKitWebContext.cpp:

‘WebKitWebContextPrivate {aka struct _WebKitWebContextPrivate}’ has no member named ‘notificationProvider’

IIRC, I could easily fix this last time by commenting out the according section. I don't know whether this is proper but assume so. I'll submit a patch a.s.a.p.

[Michael Catanzaro]

No that's not right. We support USE_LIBNOTIFY=OFF, but we do *not* support ENABLE_NOTIFICATIONS=OFF. The ENABLE(NOTIFICATIONS) guards should be removed from the GTK API layer. The only use of libnotify is in WebKitWebView.cpp and that is properly guarded.

I wonder how you wound up with ENABLE_NOTIFICATIONS=OFF? If something is setting that in our build system, it needs to be fixed.

[Michael Catanzaro]

To be clear, WebKitNotificationProvider is required for notifications to work properly regardless of whether the default libnotify implementation is compiled in or not.

[ManDay]

I do not understand your answer. What exactly is it that you're suggesting is missing? I configure WK using the following commandline

cmake -DPORT=GTK -DCMAKE_BUILD_TYPE=Release -DENABLE_ACCESSIBILITY=OFF -DENABLE_GEOLOCATION=OFF -DENABLE_INTROSPECTION=OFF -DENABLE_MINIBROWSER=ON -DENABLE_NOTIFICATIONS=OFF -DUSE_LD_GOLD=OFF -DUSE_LIBHYPHEN=OFF -DUSE_LIBNOTIFY=OFF -DENABLE_SPELLCHECK=OFF -DCMAKE_INSTALL_PREFIX="$HOME/local" -DENABLE_PLUGIN_PROCESS_GTK2=OFF -DUSE_LIBSECRET=OFF -DUSE_SYSTEM_MALLOC=ON

[Michael Catanzaro]

(In reply to ManDay from comment #3)
> -DENABLE_NOTIFICATIONS=OFF

This isn't supported, GTK+ port uses ENABLE_NOTIFICATIONS=ON. Remember back when we fixed a bunch of private options mistakenly not being marked as ADVANCED? This was one of them.

There is still a bug though, because we have ENABLE_NOTIFICATIONS guards in our GTK API code. We should either remove them, or make it public and actually allow building with it disabled.

[Michael Catanzaro]

What you can do is build with ENABLE_NOTIFICATIONS=ON USE_LIBNOTIFY=OFF, in which case applications still get callbacks when there is a web notification but have to implement the notifications themselves (or just ignore it). That's also an option for applications when using USE_LIBNOTIFY=ON. libnotify is just a default handler.

[Claudio Saavedra]

Created attachment 309707 [details]
Patch

[Build Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[WebKit Commit Bot]

Comment on attachment 309707 [details]
Patch

Clearing flags on attachment: 309707

Committed r216675: <http://trac.webkit.org/changeset/216675>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.