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

mouse standard actions #2337

Closed
jerch opened this issue Jul 24, 2019 · 4 comments
Closed

mouse standard actions #2337

jerch opened this issue Jul 24, 2019 · 4 comments
Assignees

Comments

@jerch
Copy link
Member

jerch commented Jul 24, 2019

While reworking the mouse handling and writing tests for it it became obvious that we have issues with mouse standard actions:

  • event listeners are scattered over several places
  • some standard actions are cancelled, other are not
  • escaping rules to get back standard actions are different for platforms and hardcoded

Whats needed:

  • define/establish default behavior for actions (e.g. middle click should never report clipboard when mouse is caught)
  • establish default escape rules (may always go with the same modifier to get the standard action back), ideally this will be customizable in the end so ppl/integrators can use their scheme here
  • find a better maintainable solution for event listeners (Make an event listener service?)

Refs:

@jerch jerch added area/mouse-support breaking-change Breaks API and requires a major version bump labels Jul 24, 2019
@jerch jerch added this to the 4.0.0 milestone Jul 24, 2019
@jerch jerch removed the breaking-change Breaks API and requires a major version bump label Jul 25, 2019
@jerch
Copy link
Member Author

jerch commented Aug 1, 2019

Current issues:

  • mouse middle click: This is currently hard coupled in Terminal.ts to auxclick on linux, which provides the X11 typical middle click clipboard buffer. Imho this should be detached if the mouse was caught by the terminal app, which is currently not the case. Problems here - it might confuse the stdin loop of the terminal app, worst case - it might leak sensitive data from the clipboard by accident.
    Proposal: Always detach if mouse was caught, maybe allow a modifier overwrite (+Shift), which should supress the middle click being sent to the app in reverse.
  • Shift + mouse clicks are caught by the selection manager on non OSX platforms: Imho this should be unified to same default behavior across all platforms.
    Proposal: Find a universal escape modifier for several actions, that should escape terminal mouse binding. Shift seems to be a good candidate, e.g. Shift + left button could always escape to selection manager. Those actions should never be reported to the terminal app. Additionally this modifier should be in line with common other emulators. Furthermore we might need to make this customizable so integrators can adopt their "key" here.

To overcome the currently scattered event listener handling in Terminal.ts it might be worth to establish an event listener and dispatching service under browser that deals with all types of event listener juggling, e.g. key events, mouse events and others we need. This service could provide an interface to register certain event types from inner services, e.g. an input service would request key handling, the bufferService can request buffer scrolling, the selection manager its own rules to get the selection stuff done. Same with the CoreMouseService, it would just request certain mouse events to be routed.

Why do we need such a global event listener service? Imho we have several services already competing over the the limited resource "eventXY", thus such a service can act as an dispatching gateway dealing with these aspects:

  • install default routes, e.g. key handling, buffer scrolls and selection should work from the start
  • apply custom routes, e.g. CoreMouseService requests some mouse events to be routed, this service should be able to:
    • install/change needed listeners
    • detach default routes
    • install modified event escape routing rules

Basically this reads like we are doing our own GUI event framework, still I think we need a single point of responsibility for event listeners and dispatching rules to overcome the problems we currently have here.

@mofux
Copy link
Contributor

mofux commented Aug 1, 2019

Can you elaborate on your idea with routing and rules a little more closely? I do like the evt.stopPropagation() pattern - the problem is that we currently cannot control the order in which events of the same type (e.g. mousedown) are dispatched to their registered listeners. IMO such a service could solve that problem by allowing listeners to specify a priority. So instead of this:

// selection manager
terminalElement.addEventListener('mousedown', (e) => ...)

// input handler
terminalElement.addEventListener('mousedown', (e) => ...)

We would have something like this:

// selection manager
const disposable = domEventService.addEventListener('mousedown', 10, (e) => ...)

// input handler
const disposable = domEventService.addEventListener('mousedown', 100, (e) => ...)

This way, we would dispatch the mousedown event to the selection manager first (prio 10), and only if he doesn't call e.stopPropagation() we will continue to dispatch it to the next listener (input handler with prio 100).

This would establish an ordered chain of listeners, where any listener in the chain can decide to stop the event, so it doesn't get consumed by lower prio listeners:

mousedown > selection manager > input handler > some other listener > ...

Is this in line with your idea?

@jerch
Copy link
Member Author

jerch commented Aug 2, 2019

@mofux: I have not yet thought about this in detail, but imho this could be part of the routing. The CoreMouseService.triggerMouseEvent already contains a boolean return value, that could be used to decide the further bubbling.

A priority setting would help during initial setup to get it working without caring to much about attach order from other services. Without it we would have to make sure that services register the events in the right order, with the priority setting we can juggle that in the event service itself.

Getting this wrapped into a good interface design is abit tricky, not sure yet if we should go with multiple listeners that use native event handler queueing and bubbling (preventDefault, stopPropagation and such), or if it might be easier to register just one listener for eventXY and do the routing/dispatching afterwards.

From my limited mouse event listener understanding, the latter might be easier in combination with our own custom internal events for a simple reason - the service can replace the native listeners anytime to fullfill the requested events needs, while dealing with those within the listener code directly might be hard to catch all conditions.

I can imagine that something like this would work:

  • selection service requests EventMouseDown and EventMouseDrag by default
    • EventMouseDown is simple - the service installs a 'mousedown' listener on the terminal element
    • EventMouseDrag is tricky - there is no native mousedrag event, thus the service would have to install a 'mousedown' listener that additionally catches 'mousemove' after 'mousedown' and gets removed after 'mouseup' --> already 3 listeners needed for EventMouseDrag
    • service now routes EventMouseDown and EventMouseDrag to selection service
  • now a terminal app binds to EventMouseDown --> CoreMouseService requests EventMouseDown
    • service alters route for EventMouseDown to CoreMouseService (maybe with priority settings to deal with concurrent eventXY requests)
    • selection does not get this event anymore
    • optionally the service installs a modifier escape rule to get the event routed back to selection

@jerch
Copy link
Member Author

jerch commented Feb 1, 2021

Closing this as it got stale. This might be relevant once we get to internal API-asation, but its easy to come up with better suitable ideas based on the shifted services later on.

@jerch jerch closed this as completed Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants