atlas-engineer / nyxt

Nyxt - the hacker's browser.
https://nyxt-browser.com/
9.91k stars 416 forks source link

WebKitGTK port skips `on-signal-decide-policy` for mouse-clicked links #2634

Open aartaka opened 2 years ago

aartaka commented 2 years ago

Clicking a link relays things right to the view, thus skipping any blocking, redirection, and mode toggling (basically skipping all the features Nyxt has to offer). Reloading the clicked link with the Reload button or reload-buffer command re-activates the modes, blocks or redirects the page. Seems like WebKit somehow hides the clicked link requests from us :(

In fact, link clicks fire the notify::uri signal, but don't fire the decide-policy signal. Maybe there's another signal in WebKit that can handle this URL routing?

Ambrevar commented 2 years ago

This is new to me. I'm sure that on-decide-signal-policy works when clicking on a link which leads to a new page load. If it does not, there might be a bug somewhere in our code.

Does this only happens when following anchors or on JS page modifications? If so, it's expected, and yes, notify::uri catches this change (if the JS event changes the URI).

Can you provide an example?

aartaka commented 2 years ago

Can you provide an example?

One particular example that never works is clicking on "tabs" (like "Issues", "Pull requests" etc.) of the repository or a pull request on GitHub.

Ambrevar commented 2 years ago

Yup, this particular example is a JS trick from GitHub I think. What happens if you enable noscript-mode?

hendursaga commented 2 years ago

Does this nx-router commit have anything to do with this issue?

aartaka commented 2 years ago

Does this nx-router commit have anything to do with this issue?

Yes, that's exactly what I'm talking about.

Ambrevar commented 2 years ago

To clarify: is this a regression? In my understanding, this is a limitation of WebKitGTK that's always been there. Or maybe even a limitation of the Web, because I don't know if WebKitGTK could do any better.

What can we do to fix this? It would be nice to conflate on-signal-decide-policy and notify::uri so that each page load triggers a single hook.

notify::uri is often a good place, but:

Suggestion:

Question: How do we detect redirections in notify::uri?

aartaka commented 2 years ago

To clarify: is this a regression? In my understanding, this is a limitation of WebKitGTK that's always been there. Or maybe even a limitation of the Web, because I don't know if WebKitGTK could do any better.

It's not a regression, it always was there. But it's annoying, and if we can fix it, then we should.

What can we do to fix this? It would be nice to conflate on-signal-decide-policy and notify::uri so that each page load triggers a single hook.

Yes, I'm thinking that too. It doesn't solve the problem of requesting the undesired resources, but it may at least cancel/load something else in the middle of loading to get to the page one expects.

  • it runs too late for regular page loads (if I recall correctly);

Yes, it's only after WebKit is sure in the URL being loaded, which is long after on-signal-decide-policy returned.

  • following an HTML anchor should not trigger the hook;

I mean, this is basically a matter of checking whether the URL went through on-signal-decide-policy or on-signal-load-*.

  • Add a buffer slot to remember if resource-request-hook was run (from on-signal-decide-policy).

I am tempted to somehow rely on the history here, but our history doesn't store that much information yet (see #1848), so we cannot rely on it in seeing whether request-resource-hook fired yet.

  • In notify::uri, only run the hook if:

    • resource-request-hook was not already run;
    • The new URL is not the same as the old URL, save the HTML anchor ;
    • It's not a redirection.

Question: How do we detect redirections in notify::uri?

No way to detect them there, but we can detect and record them in on-signal-load-redirected.

Ambrevar commented 2 years ago

*

I mean, this is basically a matter of checking whether the URL went through on-signal-decide-policy or on-signal-load-*.

That wouldn't work in problematic case like the aforementioned GitHub's "issues" link, right?

aartaka commented 2 years ago

Those don't go through on-signal-decide-policy, yes.

aartaka commented 1 year ago

From #2895:

I'm thinking of reloading the buffer in such a case, so that all rules and history are working properly, but will have to figure out some more details.