Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
illfygli opened this issue Mar 22, 2022 · 7 comments
Open

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

illfygli opened this issue Mar 22, 2022 · 7 comments

Comments

@illfygli
Copy link
Contributor

illfygli commented Mar 22, 2022

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
Copy link
Owner

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
Copy link
Owner

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
Copy link

rnwgnr commented Jan 2, 2023

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
Copy link

tp971 commented Oct 26, 2023

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
Copy link
Owner

@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
Copy link

tp971 commented Nov 1, 2023

@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.

@VisenDev
Copy link

Possibly related to #642 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants