fanglingsu / vimb

Vimb - the vim like browser is a webkit based web browser that behaves like the vimperator plugin for the firefox and usage paradigms from the great editor vim. The goal of vimb is to build a completely keyboard-driven, efficient and pleasurable browsing-experience.
https://fanglingsu.github.io/vimb/
GNU General Public License v3.0
1.33k stars 99 forks source link

hints don't work on some pages with webkit2gtk 2.36 #709

Open illfygli opened 2 years ago

illfygli commented 2 years ago

Version: 3.6.0 WebKit compile: 2.34.6 WebKit run: 2.36.0 GTK compile: 3.24.33 GTK run: 3.24.33 libsoup compile: 2.74.2 libsoup run: 2.74.2 Extension dir: /usr/lib/vimb

Steps to reproduce

Open a webpage, e.g. https://pinafore.social, and press f to show hints.

Expected behaviour

Hint overlay should be shown

Actual behaviour

Hint overlay is not shown; there is this output:

** (WebKitWebProcess:18644): WARNING **: 20:28:52.365: invalid page id 10

and when pressing esc to go back to normal mode, there is also:

** (vimb:18610): WARNING **: 20:28:55.368: Failed dbus method EvalJs: GDBus.Error:org.freedesktop.DBus.Error.InvalidArgs: Invalid page ID: 10

It only happens on some pages, I will try to find out more.

fanglingsu commented 2 years ago

The "invalid page id" sounds like an IPC issue. When the dbus connection to the webextension is established and the page is created in the webextension. The webextension sends the page id back to the UI Process so we can keep the relation between UI Process and the webextension. It seems that the UI process sends a request to create the hints with an page id that does not exist anymore in the webprocess (webextension).

fanglingsu commented 2 years ago

I've started working on another IPC attempt by using webkit built in JavaScript Based IPC mechanism - which should make some things easier. But I did not get it working until today.

rnwgnr commented 1 year ago

I can confirm this behaviour with the latest git (commit 8b85f98) and webkit2gtk 4.1 - build via ArchLinux AUR PKGBUILD. If you need more details please let me know.

For me it's happening on https://computerbase.de, most other sites work fine. But when a site is broken, it is always broken.

Was thinking about moving from qutebrowser to vimb, but this is blocker for me. :(

tp971 commented 8 months ago

I don't have any great insight on this code base (or on using GTK, WebKit, or DBus), but I stumbled upon this issue through a friend of mine and I think I found the problem: on some pages, webkit seems to create a new page, which calls on_web_extension_page_created in ext-proxy.c with the new page id. However, the code seems to assume that the page id never changes. I managed to hack together this "fix":

diff --git a/src/ext-proxy.c b/src/ext-proxy.c
index 84ab325..17bc7b2 100644
--- a/src/ext-proxy.c
+++ b/src/ext-proxy.c
@@ -319,6 +319,8 @@ static void on_web_extension_page_created(GDBusConnection *connection,
      * webextension. */
     c = vb_get_client_for_page_id(pageid);
     if (c) {
+        c->page_id = pageid;
+
         /* Set the dbus proxy on the right client based on page id. */
         c->dbusproxy = (GDBusProxy*)data;

diff --git a/src/main.c b/src/main.c
index 557a9fe..eadfa66 100644
--- a/src/main.c
+++ b/src/main.c
@@ -327,7 +327,7 @@ Client *vb_get_client_for_page_id(guint64 pageid)
 {
     Client *c;
     /* Search for the client with the same page id. */
-    for (c = vb.clients; c && c->page_id != pageid; c = c->next);
+    for (c = vb.clients; false && c && c->page_id != pageid; c = c->next);

     if (c) {
         return c;

This basically assumes that there is always one client and on_web_extension_page_created updates the page id. I assume to fix this properly, there needs to be another way to find the client inside on_web_extension_page_created that does not depend on the page id.

fanglingsu commented 8 months ago

@tp971 Thank you for this fix, but what's the reason for the false in fir for-loop? I think this doe nothing to solve the problem, isn't ist?

tp971 commented 8 months ago

@fanglingsu that's the hacky part: it makes it always return the first client (basically ignoring the pageid parameter). As I said, a correct way to fix this is to find another way to find the corresponding client in on_web_extension_page_created that does not depend on pageid.