RESOLVED FIXED Bug 26242
[GTK] The way we handle optional features is not quite right
https://bugs.webkit.org/show_bug.cgi?id=26242
Summary [GTK] The way we handle optional features is not quite right
Xan Lopez
Reported 2009-06-07 10:52:31 PDT
First with the keyring support, and now with the GNOME proxy support (see bug #25263), the way we are handling optional features is by providing *compile time* flags in webkit for them. I believe this is a less than optimal solution, and here's why: Suppose you maintain the webkit package for distro Foo, and want to support all webkit users with a single binary/package (a reasonable request, IMHO). Right now your only choice is to enable all options, and have applications check for every and each of them and disable, if it's possible, those they do not like. That's already a pretty bad design, but worse still the webkit library will still link with the optional libraries anyway, so for example Midori on Xfce will link through WebKit with GNOME libraries. I believe a better design is to allow applications to choose the options they want for themselves (ie, positive instead of negative API), with the default being a baseline webkit with no extra functionality. This is how the GNOME proxy feature used to work before bug #25263 (Epiphany chooses to enable it, and thus depends on libsoup-gnome. Midori, I assume, does not, and does nothing), since you could assume that webkit wouldn't have that functionality in any distro. My proposal is to revert the proxy patch, and change the keyring code and any future optional feature so that they can be shipped as modules that applications can optionally enable, allowing those who don't want to to not depend on the extra libraries (perhaps those modules could be in different packages in distros, I guess that's up to them).
Attachments
Christian Dywan
Comment 1 2009-06-07 11:59:56 PDT
The funny thing is, when I look at bug 25263, I remember a discussion in IRC about compile time GNOME features, ending with a conclusion that it's the wrong approach. But I missed that bug report completely. I completely agree that the current situation is completely silly from a packaging point of view. Currently Midori does in fact never make use of libsoup-gnome because it would have to be compiled with it, which means more than one unwanted dependency for a lot of users. And other users want it because they do have the packages anyway. I find the suggestion of separate modules very good, with a) a function that tells me if the feature is installed b) a function to enable the feature regardless of whether it is available. Maybe something like webkit_has_feature ("keyring") and webkit_set_feature ("keyring", TRUE), or alternatively webkit_web_settings_has_feature /_set_feature. Particularly using strings instead of defines would completely avoid the compile time dependency problem. Spell check would probably also be a module.
pzk5qt6ottv3z7ag
Comment 2 2009-06-07 12:50:06 PDT
I agree with you even as the reporter/patcher. After found that bug, I did try to search for some run-time solution/walkaround of that bug. But at last I found libsoup had made it a compile-time feature. So I've no choice... I think it'll be great if webkit provides a plugin-like mechanism, in order to tolerate such similar situations. But I wonder if too much overhead would be introduced (in the future), since any part of webkit could be substituted, we'll have to `check for plugin' everywhere throughout webkit.
Gustavo Noronha (kov)
Comment 3 2009-06-07 13:07:58 PDT
+1 I was happy to see that patch landed (in fact, I did not r+ it only because Jan had already worked on it, so I decided to leave it for him), but I agree we should have a better solution, and that this is a good time to design it, both as a developer and as a Debian maintainer - this would make my life much easier if designed right. So my take on this, would be: /usr/lib/webkit/1.0/modules There we would have gnome-keyring.so, soup-gnome.so. I think we want these to be global API, so not attached to webview or settings at all. So, say: webkit_use_feature(string, boolean) -> boolean Those strings would be the names of the modules, without '.so'. Our apps would be optimists - just request whatever features they would like to enable, our code would try to load the module, and if the symbols are not resolvable, return FALSE, returning TRUE when they are, and init worked OK, and they'll know which are not available through the return value. How does that sound?
pzk5qt6ottv3z7ag
Comment 4 2009-06-07 13:38:54 PDT
One question: Take the proxy thing as example, only loading new modules/functions is not enough, to enable/disable proxy-resolver, we'll need to substitute a function in webkit. How could this be done?(maybe for other compile-time feature of some depending module) Do we need some hook-like functions? Or webkit should be compiled awaring of all potential modules? (In reply to comment #3)
Christian Dywan
Comment 5 2009-08-13 15:59:29 PDT
For information, bug 28173 was resolved now that libsoup can store logins so WebKit doesn't have to depend on it. libsoup will also expose the information whether logins are saved in the future, so this issue is handled as far as GNOME Key Ring is concerned.
Xan Lopez
Comment 6 2009-08-26 22:49:34 PDT
I believe we can close this for now, since I don't see any reason why we should add optional dependencies of the keyring style in the future. In the event that we feel the need to do so, we can revisit this bug :)
Note You need to log in before you can comment on or make changes to this bug.