atlas-engineer / nyxt

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

Electron input handling #3381

Closed jmercouris closed 1 month ago

jmercouris commented 2 months ago

Description

Checklist:

jmercouris commented 2 months ago

@aadcg There are two commits here, both show two different ways of handling the new challenges faced with synchronous input handling in electron. The bottom line:

WE CANNOT execute ANY JavaScript when handling input from the renderer. We need to return t/nil first AND THEN execute whatever JavaScript that occurs as a result of handling input.

If we do not do this, the system will deadlock.

Why?

  1. JavaScript is waiting for a synchronous response from us as to whether it should prevent an event from bubbling, or consume it. In other words, JavaScript needs to know whether it needs to run event.preventDefault().
  2. In our input handling on the Lisp side, if we try to execute some JavaScript before we return t/nil we'll send a request to a blocked JavaScript program (the one waiting for synchronous input), asking it to do something.
  3. Our Lisp program will not continue until the JavaScript we tried to execute runs.
  4. Our Lisp program will be unable to return t/nil.
  5. We have deadlocked our program.

I hope the above makes sense. It is kind of hard to write out :-)

aadcg commented 2 months ago

@jmercouris I think I understood the ideas. If I'm not mistaken, you're still trying out different approaches so I'll review when ready.


Please limit you summary commit messages to 50 chars.

(source) A note on commit messages: Though not required, it’s a good idea to begin the commit message with a single short (no more than 50 characters) line summarizing the change, followed by a blank line and then a more thorough description.

jmercouris commented 2 months ago

I will keep this in mind, and whenever reasonable, will limit my commit message summaries!

jmercouris commented 2 months ago

@jmercouris I think I understood the ideas. If I'm not mistaken, you're still trying out different approaches so I'll review when ready.

I am ready to continue with one approach, I would like your opinion about which one it should be. Do you have any suggestions about what you think?

aadcg commented 2 months ago

I'm not sure I understand the consequences of each of the 2 approaches. The one from commit 519d38065 seems closer to how it works in GTK. The other one, from commit d058c177e, seems odd in the sense that consume-input-event-p and dispatch-input-event need to coexist, even though they're cognitively connected.

So, my question would be: in what way is d058c177e superior to 519d38065?

aadcg commented 1 month ago

For future reference, {sly,slime}-lisp-implementations has been set to

(nyxt-nix
 ("nix-shell" "shell.nix" "--run" "sbcl --dynamic-space-size 3072")
 :directory "~/common-lisp/nyxt/build-scripts/")

@jmercouris I took the approach that doesn't block on input dispatch. Seems to be working well.

Unfortunately, I wasn't able to ensure that it doesn't break anything on the WebKitGTK port since the dev env is broken on both Guix and Nix (seems related to WebKitGTK).