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

Various fixes, zoom support, dependencies for real terminal emulation #20

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ajbouh
Copy link
Contributor

@ajbouh ajbouh commented Aug 1, 2018

No description provided.

@ajbouh
Copy link
Contributor Author

ajbouh commented Aug 29, 2018

@tolmasky ping

@tolmasky
Copy link
Owner

Hi I'm just about to jump on a plane but didn't want this to go unresponded to. I'm going to try my best to look at this this week. Thanks!

@ajbouh
Copy link
Contributor Author

ajbouh commented Aug 29, 2018

Great, thanks!

I have more changes that make it easier to sync terminal keyboard events with terminal output. That way you can avoid the terminal falling behind your typing and ruining the line wrapping.

I've also been experimenting with getting hover events to show up properly during simulated mouse movement. Looking forward to getting your perspective on that as well.

@ajbouh
Copy link
Contributor Author

ajbouh commented Oct 12, 2018

@tolmasky still interested in merging this? I have a separate set of patches based on this that updates to electron v3

@tolmasky
Copy link
Owner

Hi abjouh, sorry for the delay -- I think if we can split these up a bit, it would help me taking it in, since currently whenever I have a little time, I start and never finish reviewing it all.

Specifically, it would be nice to split easy fixes like Electron updates, from "UI fixes" like the zoomLevel stuff, to potentially security affecting fixes like adding node-pty (where if you run demokit with that on it could potentially allow arbitrary websites to gain access to your computer).

One of the ideas of DemoKit is that you might want to run it in CI for example, and I'd want to at the very least significantly document the dangers of turning on node integration if we have that feature since some people may not know what that allows.

@ajbouh
Copy link
Contributor Author

ajbouh commented Oct 22, 2018

Reduced these to the four simplest ones. Will push other PRs for other features individually.

These are needed for real xterm.js support
This, together with xterm.js is needed for actual terminal
emulation.
Using mutating an element that's already in the DOM is a source of very
confusing behavior in newer versions of Electron and Chrome.

In some cases it results in two separate instances of the DOM elements
loaded within the webview. Instead we use DOM.createElement(...).
This makes it easier to debug operations that appear to hang when
they fail to "resolve" before the page unloads. We also set a
a "transient" to true in the object passed to rejection. User-level
code can check for transient === true and retry accordingly.
Switch from mouse event logic in mouse.mm to Electron's own
sendInputEvent API. This appears to be more reliable and
makes it easier to send additional events that simulate hover
events.

We now send mouseEnter, mouseMove events after completing
a move. If two successive mouse moves are in two different
windows, we now send a mouseLeave event to the previous window.

Also support double-click events by overloading the click
argument to support integer values. To simulate a double-click,
use {click: 2, ...}
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

Successfully merging this pull request may close these issues.

2 participants