| Summary: | [GTK] Remove notifications guards from GTK API layer | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | ManDay <manday> | ||||||
| Component: | WebKitGTK | Assignee: | Claudio Saavedra <csaavedra> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | berto, bugs-noreply, buildbot, cgarcia, commit-queue, csaavedra, gustavo, mcatanzaro | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
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. To be clear, WebKitNotificationProvider is required for notifications to work properly regardless of whether the default libnotify implementation is compiled in or not. 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 (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. 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. Created attachment 309707 [details]
Patch
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 Comment on attachment 309707 [details] Patch Clearing flags on attachment: 309707 Committed r216675: <http://trac.webkit.org/changeset/216675> All reviewed patches have been landed. Closing bug. |
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.