luakit icon indicating copy to clipboard operation
luakit copied to clipboard

Problems with webkitgtk 2.36

Open Wibjarm opened this issue 2 years ago • 6 comments

Running luakit with any webkitgtk release in the 2.36 series as packaged on Arch Linux (tested against 2.36.0 through 2.36.7) leads to an assert failing when going back a page in the history. This happens with both the most recent tagged release and current develop commit.

luakit 2.3.1
  built with webkit 2.36.7 (installed version: 2.36.7)
luakit 2.3.1-4-ga8b1cd5c
  built with webkit 2.36.7 (installed version: 2.36.7)
ERROR:widgets/webview.c:413:load_changed_cb: assertion failed: (!d->cert)
Bail out! ERROR:widgets/webview.c:413:load_changed_cb: assertion failed: (!d->cert)

I can reproduce this semi-consistently by going to https://www.archlinux.org, navigating to the forums, viewing any category, then hitting the previous page keybind or navigating back from the context menu, and happens with a completely stock config file.

There's also an issue with link hints not working on certain pages, but there aren't any visible errors to work from there, there are just no link hints created at all.

Wibjarm avatar Sep 01 '22 19:09 Wibjarm

I'm not able to reproduce this with current develop under Debian Sid:

# luakit --version
luakit 2.3-101-g69128e11
  built with webkit 2.34.1 (installed version: 2.36.7)

Would you fill in the rest of the details requested in the new issue template?

taobert avatar Sep 02 '22 12:09 taobert

$ luakit --version
luakit 2.3.1
  built with webkit 2.36.7 (installed version: 2.36.7)

I opened https://bbs.archlinux.org, then opened the "Newbie Corner" and then went back in history with the keybinding Shift+H. I also tried the context menu -> back via mouse.

No problem here on OpenBSD 7.2-beta.

c0dev0id avatar Sep 02 '22 15:09 c0dev0id

Sorry, I'd missed the issue template somehow, edited with more details. Unfortunately the repro is also weirder than I thought, it only crashes if I've viewed the main archlinux.org frontpage before hitting bbs.archlinux.org, which is an interesting twist.

Wibjarm avatar Sep 02 '22 18:09 Wibjarm

Thanks! I can reproduce it now.

c0dev0id avatar Sep 02 '22 19:09 c0dev0id

I don't understand what's happening at this exact code path. But when I remove the assertion and move tls_get_info behind the if-check, the navigation works.... sort of.

--- widgets/webview.c.orig      Fri Sep  2 21:36:00 2022
+++ widgets/webview.c   Fri Sep  2 21:35:51 2022
@@ -410,10 +410,10 @@
             d->cert = NULL;
         }
     } else if (e == WEBKIT_LOAD_COMMITTED) {
-        g_assert(!d->cert);
-        webkit_web_view_get_tls_info(d->view, &d->cert, NULL);
-        if (d->cert)
+        if (d->cert) {
+            webkit_web_view_get_tls_info(d->view, &d->cert, NULL);
             g_object_ref(G_OBJECT(d->cert));
+        }
     }

     if (e == WEBKIT_LOAD_COMMITTED)

When I do so, I notice that the loading indicator stays at 10% at the critical navigation step. I can navigate back to the homepage and the indicator still stays at 10%. If I navigate forward again inside a forum topic at bbs.archlinux.org, the situation fixes itself up and we're good again.

c0dev0id avatar Sep 02 '22 19:09 c0dev0id

I still can't replicate this, but i think your logic's wrong. We're not asserting that d->cert is set to something, we're asserting that it's NULL. We then get the certificate with webkit_web_view_get_tls_info() and if that succeeds, then we increase the reference count to stop it getting freed.

The other place webkit_web_view_get_tls_info() is called is in luaH_webview_ssl_trusted() which has a comment stating: "make sure this function is called after WEBKIT_LOAD_COMMITTED" So it might be an issue with the ordering of these two calls, it might be that load_changed_cb() is somehow called twice, or it might be that d->cert is not cleared correctly after navigating to a new page. The first seems unlikely as luaH_webview_ssl_trusted() sets a local GTlsCertificate *cert; rather than d->cert. Also, in my hands, adding info()s to load_changed_cb() and luaH_webview_ssl_trusted() doesn't reveal odd ordering, or multiple calls to load_changed_cb() (but as i mentioned, i'm also not seeing the assertion fail). I do see occasional multiple calls to luaH_webview_ssl_trusted(), but this may be innocuous.

The NULL which is passed in as the third parameter to webkit_web_view_get_tls_info() can optionally be a pointer to an integer to return error codes (as is done in luaH_webview_ssl_trusted()). This might be useful, but in my hands it is consistently set to 0.

To get this to even load arch's page, i need the patch i mention in #1017. I also see the page stopping at 10%, but a reload seems to correct this. When i see the 10% it seems to correlate with this patch complaining about "webview_get_endpoint called with bad widget". I don't know whether this is pertinent. Potentially pertinent though, is that it also seems to correlate with not seeing the "I [lua/webview]: Requested link: ..." line printed one navigation previous. That is, if i'm on https://archlinux.org/ and click "forums" i see "I [lua/webview]: Requested link: https://bbs.archlinux.org/ (text/html)". If i then press "Newbie Corner", i see "I [lua/webview]: Requested link: https://bbs.archlinux.org/viewforum.php?id=23 (text/html)". If i then press back, i don't see "I [lua/webview]: Requested link: https://bbs.archlinux.org/ (text/html)", although the navigation succeeds. if i press back again, i see "E [core/widgets/webview]: webview_get_endpoint called with bad widget" followed by "I [lua/webview]: Requested link: https://archlinux.org/ (text/html)", and although the load completes, the status bar sticks at 10%.

taobert avatar Oct 11 '22 08:10 taobert