Upgrade to WebKit2
Closed this issue · 48 comments
WebKit 1 has security issues that won't be resolved. Please consider upgrading to WebKit2.
See https://blogs.gnome.org/mcatanzaro/2016/02/01/on-webkit-security-updates/
I've looked into this (see webkit2 branch.) Upgrading would fix #315.
Some things are broken :
WebKit 2 uses global proxy settings, there is no API to set a custom proxy :
WebKitWebView makes its own scrollbars and doesn't implement GtkScrollable anymore. The scrollbars are not in the same style as other scrollbars in the application. And the headlines skimming functionality (Ctrl+Space) is broken.
The functions to check if something is selected are gone, so the "Copy" action in the context menu is always active.
@Leiaz The feature breaking make this a hard upgrade. I could live with the "Copy" action missing, but the skimming is not nice.
It can probably be made to work by making a Web Extension for WebKit and using the DOM API.
Epiphany can serve as example, they use DBus to communicate with the extension.
Argh... this cannot be right, everyone using Webkit2Gtk needs to do this? Then it belongs into Webkit2Gtk...
The scrollbars are not in the same style as other scrollbars in the application.
I guess this is fixed with latest >=webkit-gtk-2.11.90[1]
- Switch to use overlay scrollbars like all other GTK+ widgets and ensure the behavior is consistent with GTK+ too.
[1] http://webkitgtk.org/2016/02/19/webkitgtk2.11.90-released.html
@Leiaz I tried buiding this, but I suspect I'm doing something wrong as I get blank window where the feed item is displayed. Is there a minimum webkit2-gtk needed for this to work?
Update: I updated libgtk gtk3-devel glibmm2-devel libXinerama-devel libXrandr-devel and wayland-devel and got it to work.
I'm using webkit2gtk 2.10.7, but 2.6 should be enough. I'm not seeing those errors. Which version are you using ?
I had webkit2gtk 2.6.6 installed, but after updating to 2.10.7, still had issues because the dependencies had not installed. I also had issues, because I had older versions of some webkit/pango dependencies installed in /usr/local.
Some observations on the webkit2 port.
(1) Control-space no longer scrolls the current article, but acts like 'next' when the article is complete.
(2) If the displayed page crashes, the liferea app doesn't crash.
(3) I have not had any javascript errors. Still testing. So far so good.
I've made a web extension and now Control+space and the copy menu item (should) work.
As noted in commit comments there is webkit_web_view_can_execute_editing_command
that can be used to check if copy is to be enabled, but it is asynchronous so I don't think I could use it without changing the interface with LifereaHtmlView (?). So I'm using the web extension.
Control-space is working. Thank you.
I noticed this message before, during startup and when a picture paints, and didn't log it before.
(WebKitWebProcess:24372): GLib-GObject-WARNING **: gsignal.c:2516: signal 'starting' is invalid for instance '0x21659b0' of type 'SoupMessage'
@rich-coe This is from WebKit, the doc here says this signal is new to libsoup 2.50.
I've made some changes to do the menu with actions and use asynchronous calls.
The web extension is now only used for headlines skimming. It could be done in Javascript instead but this would require having Javascript enabled ...
I've removed the proxy settings. I wanted to change the deprecated GtkVBox GtkHBox to GtkBox but then I saw the comment in the doc saying that GtkGrid should be used because GtkBox is going to be deprecated too. So I didn't do it but it may be simpler to remake the ui files in the new Glade.
@lwindolf Do I send a pull request or do you prefer Javascript to a web extension ?
Note that we only connect to SoupMessage::starting if compiled against sufficiently-new libsoup. The only way you could be seeing that error is if your WebKit was compiled against a newer version of libsoup than you have installed (a serious distro bug).
It was added here: http://trac.webkit.org/changeset/180928
@mcatanzaro I upgraded libsoup and that fixed it. I had upgraded webkit from a new repository. Unfortunately rpm is not very good at dependencies.
For some time I'm trying to make this run on my system, but it is always stuck at startup, not sure if this is a relevant part, but it may be, it tries to "move"/"rename" a file which doesn't exist. I tried to remove all confings/cache ... ~/.cache/liferea ~/.local/share/liferea ~/.config/liferea
close(30) = 0 munmap(0x7fc828000000, 4096) = 0 rename("/home/branko/.config/liferea/feedlist.opml~", "/home/branko/.config/liferea/feedlist.opml") = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 2994) = 0 (Timeout) recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 9997) = 0 (Timeout) recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 9879) = 0 (Timeout) close(34) = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 109) = 0 (Timeout) recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 1026) = 0 (Timeout) close(29) = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 87) = 0 (Timeout) close(28) = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 328) = 0 (Timeout) close(35) = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 256) = 0 (Timeout) close(27) = 0 recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 8300) = 0 (Timeout) recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable) poll([{fd=3, events=POLLIN}, {fd=5, events=POLLIN}, {fd=10, events=POLLIN}, {fd=22, events=POLLIN}], 4, 9994q) = 1 ([{fd=5, revents=POLLIN}]) recvmsg(5, {msg_name(0)=NULL, msg_iov(1)=[{"U\2\f\1=~U\0\3\24\4\0\20\0\0\0\0\0\0\24\24\24\24\24\0\0\3\37%\2\0\0", 4096}], msg_controllen=0, msg_flags=0}, 0) = 32 recvmsg(5, 0x7ffdd5ab45d0, 0) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(5, 0x7ffdd5ab45b0, 0) = -1 EAGAIN (Resource temporarily unavailable)
The file ~/.config/liferea/feedlist.opml~
is only supposed to exist temporarily and this line :
rename("/home/branko/.config/liferea/feedlist.opml~", "/home/branko/.config/liferea/feedlist.opml") = 0
Tells you the return value was 0, which means it succeeded.
One reason Liferea may fail to start is if you didn't install it, it can't find the correct GSettings schema. Otherwise can you post the gdb backtrace ?
Thanks @Leiaz, it is built locally and installed locally, I used --prefix=/home/branko/bin and I installed it locally. As for Gsettings schemas, I did have issues with them, but I found a solution
GSETTINGS_SCHEMA_DIR="/home/branko/bin/share/glib-2.0/schemas/"
without it it "crashes" (Trap) at startup.
As for gdb, I don't have debug symbols, maybe I can rebuild some things and have debug symbols
#0 0x00007fece1db51cd in poll () from /lib64/libc.so.6 #1 0x00007fece22d6eec in ?? () from /usr/lib64/libglib-2.0.so.0 #2 0x00007fece22d6ffc in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0 #3 0x00007fece2af6c0c in g_application_run () from /usr/lib64/libgio-2.0.so.0 #4 0x000000000041a856 in main (argc=1, argv=0x7ffcc1fa0808) at main.c:258
Liferea symbols should be enough for most things.
But I just realized, you mean it doesn't actually crash ? If you were using 1.10 before, the tray icon is now a plugin, so your window may be hidden.
Ok, that was the issue, I was thinking about it before, but I excluded it as a problem when I removed my current config.
It works now with '-w shown'.
Thank you @Leiaz.
I must say that his port works really good for me. There is only one minor issue, sometimes when you start liferea (in my case it's in systray) and update feeds (maybe not important) try to open one news item there is nothing in the webview (or however it is called), just blank, as a workaround I change view mode (Normal, Wide, Combined), switch from one to another (it starts working), and back to preferred one "Normal". I'm still on webkit 2.10.x and gtk+ 3.18.x, I'll try updating webkit to 2.12. I guess it will work with other "older" libs...
After upgrading to latest stable webkit, gtk+, ... it is much better experience.
I would love to see this "merged" into official repos, and being updated, it works very well, and stable for me. Not to mention advantages of using modern webkit with bug and security fixes.
Thank you @Leiaz for this port, and thanks to @lwindolf and all contributors for liferea.
Hm, when choosing between a rarely used feature and getting a bunch of known security bugs fixed I'd choose the latter.
I believe it would be best if proxy settings were set system-wide. Should every app that uses the Internet have its own proxy settings dialog? Because I think that's probably a really bad experience for users.
For the record, the current situation:
- glib-networking uses GNOME system settings (GSettings via GProxyResolverGNOME) if GNOME_DESKTOP_SESSION_ID is present in the environment; this should be sufficient for all GNOME users unless you want to use a different proxy just for Liferea for some reason.
- Otherwise, glib-networking uses libproxy (via GLibProxyResolver) which just checks the FTP_PROXY, HTTPS_PROXY, and HTTP_PROXY environment variables.
So the system-wide configuration (which is definitely best) should already handled satisfactorily.
I am not sure why anyone would want to use different proxy settings just for Liferea -- we would certainly never expose such settings in a GNOME app, for the reason @jbicha suggests -- but my understanding is that's the only missing feature here. So what exactly is needed from WebKit? Just a way to set the proxy-uri property of the SoupSession in the network process? If so, that shouldn't be too hard, it's just a matter of someone finding time to work on it and get a patch adding that new API through the WebKit review process. But if you want to be able to use a custom GProxyResolver, that's not going to be technically possible unfortunately.
@mcatanzaro : When using e.g. Liferea with Tor, setting proxy through GNOME or through environment variables is a bad idea if you also want to use web services which will leak private data (e.g. gnome weather app (by design) or any browser on some websites or gnome-contacts with this bug). Users might want to separate those pieces of information from their RSS feeds.
Also, many provies (including Tor), are known to attack users. Using RSS through them would be ok, but not something like evolution or your web mail account.
@mcatanzaro Another use case are the non-GNOME users (Windows, XFCE, Enlightenment). Dropping the manual proxy setting would remove usability a lot for those.
@jbicha I agree the UX is not perfect. Still Liferea is not a GNOME application and cannot rely on the GNOME defaults. We workaround the UX issues by closely following the major dialog layouts (GNOME and Firefox) while defaulting to the GNOME settings.
I believe we need to follow the environment variable hack to workaround, as implemented by Nuvola (https://github.com/tiliado/nuvolaplayer/blob/c01dd44d2cd1f7114ecd5bf5db7311c9850a107f/src/nuvolakit-runner/webkit/WebEngine.vala) as suggested in (https://bugs.webkit.org/show_bug.cgi?id=128674). It should have low impact on complexity and is reasonable to test.
This way we can switch to recent Webkit.
@Leiaz I'll merge your implementation soon. I hope you do not mind.
Nuvola's hack doesn't look safe, we've learned from crashes in GNOME that you can't modify the environment in a threaded program after creating your first secondary thread, as it's impractical to ensure the other threads will never access it at the same time. (setenv needs to be done at the very top of main, or it's almost surely not safe.) It'd be hard to do with a preferences dialog anyway. I think we just need to add some API along the lines of webkit_web_context_set_proxy_uri
to WebKit.
@mcatanzaro Yes it would be enough, setting the proxy-uri of the SoupSession is how it is done with WebKit 1.
@lwindolf I had removed the proxy settings, as the environment variable thing is a bit hacky and the users who want a different proxy for Liferea could set the env variables themselves (but yes it is less user friendly than a GUI). I think setting it the way Nuvola does will require restarting Liferea for changes to take effect.
You'll have to rebase or cherry-pick to remove the commits where I removed the proxy settings. There is also this pull request which I haven't merged.
The youtube video at embedded in this web page
https://www.geeksaresexy.net/2016/07/27/internet-ads-salesmen/
kills the webkit code.
I think it's a webkit issue, but I haven't tried it yet with the standalone webkit viewer.
webkit2gtk (4.0 I believe)
I have a WIP patch for WebKit in https://bugs.webkit.org/show_bug.cgi?id=128674, will try to finish it soonish.
I updated the WebKit bug with a new patch. Please test it out and see if it's sufficient for Liferea.
Sorry for not having noticed this sooner, but there is currently an inconsistency in the way Liferea behave (master version at least). Liferea has three proxy settings : Auto Detect (use environment setting), No proxy or Manual Setting.
The proxy is configured by setting the SoupSession proxy-uri property. Setting this property has the side effect of unsetting the default proxy-resolver property of the SoupSession.
For both feed downloading and WebKit, when the user starts Liferea with either Auto Detect or No Proxy selected, proxy-uri isn't modified and the default proxy resolver is used. If the user changes the setting to either Auto Detect or No Proxy, the proxy-uri is set to NULL, which disables the default proxy resolver until Liferea is restarted.
@lwindolf Perhaps proxy settings could be reduced to two options: Environment Default or Manual Setting ? This would work nicely with @mcatanzaro patch.
The branch webkit2-with-proxy is my webkit2 branch without commits that removed proxy settings. The branch webkit2-with-proxy-patch uses @mcatanzaro 's patch.
Hm. We could add a way to guarantee no proxy will be used (perhaps by passing "direct://" as the proxy URI?) That seems like it might be useful to have and easy to implement; what do you think? (It would unset both the proxy-uri and proxy-resolver properties of the SoupSession.)
Although, I wonder if, considering the fact that nobody noticed the existing Liferea option was broken until now, maybe this setting is not so important.
With the api proposed on the bug page it shouldn't be a problem to (fix and) keep the option.
The "Scrollbars have different style in WebKit2" issue is tracked here: https://bugs.webkit.org/show_bug.cgi?id=161735
Note that we didn't know about the scrollbar problem until today, since nobody ever reported it and we only test Adwaita.
The scrollbars work fine here since 2.12 (or around that). They just weren't styled before. That bug is different.
On KDE Plasma 5, there's some UI widget that overlays the gtk scrollbar. I'm not sure why or what problem it's attempting to fix. It's ugly also.
Does it look better in KDE if you run with GTK_OVERLAY_SCROLLING=0 in the environment?
If other GTK+ 3 apps do not experience this issue, please file a new WebKit bug separate from Milan's.
@mcatanzaro GTK_OVERLAY_SCROLLING=0 just makes the ugly scoll bar appear all the time instead of auto-hiding.
I've merged @Leiaz Webkit2 implementation. @Leiaz I marked all as GPLv2+ with you as author. Can you give me an "OK" for this please?
@ALL: I do not have scroll bar issues in GNOME and Unity, but will continue watching out for those.
Issues that need fixing before 1.12 though:
- Article skimming broke
- Very small font size on my system (maybe a HiDPI problem)
- a Webkit2 version dependant use of the new proxy (#ifdef version > x.y and stdout info message otherwise)
Nonetheless I will close this ticket as it is about the Webkit2 migration and track the other issues as 1.12 integration tickets. Please everyone start posting tickets on
@lwindolf OK. Another possible issue : I noticed that I'm using Glib macros that avoid a lot of boilerplate for GTypes but are new to glib 2.44. Current Debian stable (8 Jessie) is only at 2.42, but it has webkit2gtk-4.0. Do I make it work on Debian stable ?
For article skimming I'm afraid that webkit_dom_dom_window_scroll_by
could be asynchronous ...
Regarding small fonts: the font size API is terrible to use and needs to be replaced, but in the meantime you have to handle it manually to get hidpi right. There is some boilerplate code you can copypaste from epiphany for this. See https://bugs.webkit.org/show_bug.cgi?id=142673 and https://git.gnome.org/browse/epiphany/tree/embed/ephy-embed-prefs.c#n206
Regarding proxy support: I'll try to have a new WebKit patch ready in the next week or two; my previous API proposal was rejected.
The proxy API has landed in WebKit; it will be available in WebKitGTK+ 2.16. Better late than never!
Great! Thanks for letting us now @mcatanzaro !