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

change on menu activation on list #6003

Closed
Ducasse opened this issue Mar 26, 2020 · 4 comments · Fixed by #6039
Closed

change on menu activation on list #6003

Ducasse opened this issue Mar 26, 2020 · 4 comments · Fixed by #6039

Comments

@Ducasse
Copy link
Member

Ducasse commented Mar 26, 2020

May be this is not a bug.
Now I noticed that in Epicea if I select a list of items and select one action, the action is only happening on the items currently getting the action (because when I select the menu item it deselect the list).
I experimented a bit and to have the action on the selected items I have to keep shift or option pressed.

And I wonder if this is not related to the event change.

@hogoww
Copy link
Contributor

hogoww commented Mar 27, 2020

I was about to open a new issue, but since Steph already did, I complete it with what I already written:

I don't know whether that should be reported here, or on the spec repository.

description
Cannot open menu without loosing items selected in SpPresenterList
Witnessed with Epicea.
On a fresh pharo 9 image.

reproduce
Make several changes to an image and quick without saving
Open it again, and use the code changer.
Try selecting several items, maybe with ctrl+a
right click -> menu opens but selection was lost.

Expected behavior
Right click menu open, and selection is preserved

Version information (please complete the following information, you can get it from Help->about):

@guillep
Copy link
Member

guillep commented Mar 27, 2020

Yes, this is a bug, probably related to the modal changes in menus.
It seems we're losing the multiple selection?

I'm checking it now.

@guillep
Copy link
Member

guillep commented Mar 27, 2020

The issue was effectively introduced in build 178.

The PR: #5964

So this has something to do with the event management, not the world refactoring.

@guillep
Copy link
Member

guillep commented Mar 27, 2020

I've found the real problem with the event/click/button. Super funny hazardous thing.

How Mouse Events Work

The Hand is the one managing/dispatching the events.
When the Sensor (running in a higher priority thread) finds events, it puts them into a queue.
Eventually, the hand dequeues events and treats them.

Now, the mouse events have two main flavours: mouseDown and mouseUp
There is no click, nor double click event, they are simulated by the hand morph.

When a morph is interested on a click it has to

  1. subscribe to mouse down first
  2. when it receives a mouse down, it has to ask the hand "Hey! I would like clicks!"
    Usually done by some code like this:
anEvent hand waitForClicksOrDrag: h
  event: (anEvent transformedBy: tfm)
  selectors: { nil. nil. nil. #dragTarget:. }
  threshold: 5

When that happens, the hand changes its state, and stores a "MouseClickState object" that implements a kind of state machine to detect clicks and double clicks.

After the mouseDown, the morph receives a mouseUp (that should be transformed into a click).
So the hand, before sending the event to the morph, it makes it pass through the state machine which has some code like this:

self click.    
aHand handleEvent: firstClickUp.

Where

  1. click will send a click event to the morph and
  2. handleEvent: will send the mouseUp

The problem

One problem happens when a morph decides to BLOCK on a modal window on the click- 😄
That means that mouseUp event will stay blocked in the stack, because the click does not return from the modal until modality finishes.
This means handleEvent: is never sent

In my fix the other day, I exchanged the order.

aHand handleEvent: firstClickUp.
self click.

That solved that first problem. But that alters the order of the events.
So morphs used to receive MouseDown, Click, MouseUp now receive a MouseDown, MouseUp, then Click. This makes FastTable event logic loose the multiple selection.

Thinking on a solution

Changing the order like I did above is not a good solution because It the event order makes sense.
In any case, reverting it will move the problem to mouseUp: If somebody tries to use a modal window on mouseUp => Boom same problem.

The solution has to ensure that:

  1. the order of events is the right one
  2. no events get "blocked"

it looks like we have to "return" the event to the event queue before clicking.
In case click blocks modally, the next event cycle will still handle the "returned" mouseUp.

Other Pointers

The code of MouseClickState>>handleEvent: evt from: aHand seems strange. The original event is only handle through handleEvent: in one case. But other cases will still generate clicks and double clicks, but not dispatch the original event.

This should probably be tested further and deserves its own issue.

guillep added a commit to guillep/pharo that referenced this issue Mar 27, 2020
 - refactor processEventsFromQueue:, split obtention of events in nextEventFrom:
 - add pendingEventQueue and queuePendingEvent:
 - make nextEventFrom: give priority to pending events
 - make MouseClickState queue pending mouseUp for next cycle
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 a pull request may close this issue.

3 participants