RESOLVED FIXED 170014
[GTK] Remove notifications guards from GTK API layer 
https://bugs.webkit.org/show_bug.cgi?id=170014
Summary [GTK] Remove notifications guards from GTK API layer
ManDay
Reported 2017-03-23 11:36:17 PDT
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.
Attachments
Error as produced by cmake ; make (792 bytes, text/plain)
2017-03-23 11:36 PDT, ManDay 		no flags
Details
Patch (2.15 KB, patch)
2017-05-11 04:57 PDT, Claudio Saavedra 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2017-03-23 13:03:15 PDT
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
Comment 2 2017-03-23 13:04:15 PDT
To be clear, WebKitNotificationProvider is required for notifications to work properly regardless of whether the default libnotify implementation is compiled in or not.
ManDay
Comment 3 2017-03-23 23:40:34 PDT
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
Comment 4 2017-03-24 05:59:14 PDT
(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
Comment 5 2017-03-24 06:00:40 PDT
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
Comment 6 2017-05-11 04:57:09 PDT
Created attachment 309707 [details] Patch
Build Bot
Comment 7 2017-05-11 04:59:10 PDT
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 8 2017-05-11 05:54:29 PDT
Comment on attachment 309707 [details] Patch Clearing flags on attachment: 309707 Committed r216675: <http://trac.webkit.org/changeset/216675>
WebKit Commit Bot
Comment 9 2017-05-11 05:54:30 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.