Smalltalk › Squeak › Squeak - Dev

Bug in #tryInvokeHalo:

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

6 messages Options

Stéphane Rollandin

Bug in #tryInvokeHalo:

Hello all,

To see a bug in the halo targets dispatch mechanism, do the following:

- Open a fresh image, use the alphabetical list from the "new morph..."
item in the World menu to get a SameGame morph.

- Put the SameGame morph above the "Welcome to Squeak" window.

- Blue click three times in a row within the SameGame inner part: this
will give a halo to one of the small square tiles.

- Now blue click on the grey border of the SameGame morph (say, at
left): instead of that morph getting the halo, it will go to the
"Welcome to Squeak" window in the back. That is the bug.

Reverting #tryInvokeHalo: in PasteUpMorph to its previous version fixes
it (the current implementation discards the first morph in the targets
stack, which happens in this case to be the target we do want).

Stef

marcel.taeumel

Re: Bug in #tryInvokeHalo:

Hi Stef,

I recall a discussion about this halo-dispatch behavior with Chris not too long ago. We do need tests for this halo dispatch to both document and verify the desired behavior.

Maybe we can try list and discuss the desired dispatch here, then we can try to write tests for it. (Blue-click) event simulation is not that hard. See UserInputEventTests >> #blueMouseDownAt: and MorphicEventTests.

Best,

Marcel

Am 24.04.2019 14:13:17 schrieb Stéphane Rollandin <[hidden email]>:
Hello all,

To see a bug in the halo targets dispatch mechanism, do the following:

- Open a fresh image, use the alphabetical list from the "new morph..."
item in the World menu to get a SameGame morph.

- Put the SameGame morph above the "Welcome to Squeak" window.

- Blue click three times in a row within the SameGame inner part: this
will give a halo to one of the small square tiles.

- Now blue click on the grey border of the SameGame morph (say, at
left): instead of that morph getting the halo, it will go to the
"Welcome to Squeak" window in the back. That is the bug.

Reverting #tryInvokeHalo: in PasteUpMorph to its previous version fixes
it (the current implementation discards the first morph in the targets
stack, which happens in this case to be the target we do want).

Stef

Chris Muller-3

Re: Bug in #tryInvokeHalo:

In reply to this post by Stéphane Rollandin

Hi Stef,

I just tried those steps. I can reproduce the problem when I click in
the *gray area* (e.g., parent-most morph) on the right side of the
halos, but when its in that same gray area on the left hand side, or
any other morph like the Help button, it works. Very strange. There
must be something unique about this particular "Same" game's Morph
structure or w.r.t. how it interacts with the new implementation of
#tryInvokeHalo:. This is the first Morph I've seen behave this way.

The problem with reverting the change to tryInvokeHalo: is that it
reverts what that change fixed. I think it won't be very hard to step
through that in the debugger and see why its happening. I'll try to
take a look tomorrow or this week.

I also support Marcel's idea of defining how we want it to work in some tests.

Best,
Chris

On Wed, Apr 24, 2019 at 7:13 AM Stéphane Rollandin
<[hidden email]> wrote:

>
> Hello all,
>
> To see a bug in the halo targets dispatch mechanism, do the following:
>
> - Open a fresh image, use the alphabetical list from the "new morph..."
> item in the World menu to get a SameGame morph.
>
> - Put the SameGame morph above the "Welcome to Squeak" window.
>
> - Blue click three times in a row within the SameGame inner part: this
> will give a halo to one of the small square tiles.
>
> - Now blue click on the grey border of the SameGame morph (say, at
> left): instead of that morph getting the halo, it will go to the
> "Welcome to Squeak" window in the back. That is the bug.
>
> Reverting #tryInvokeHalo: in PasteUpMorph to its previous version fixes
> it (the current implementation discards the first morph in the targets
> stack, which happens in this case to be the target we do want).
>
>
> Stef
>

Stéphane Rollandin

Re: Bug in #tryInvokeHalo:

> I just tried those steps. I can reproduce the problem when I click in
> the *gray area* (e.g., parent-most morph) on the right side of the
> halos, but when its in that same gray area on the left hand side, or
> any other morph like the Help button, it works. Very strange. There
> must be something unique about this particular "Same" game's Morph
> structure or w.r.t. how it interacts with the new implementation of
> #tryInvokeHalo:. This is the first Morph I've seen behave this way.

No, the bug is very general - I just gave fast and easy steps to
reproduce it. But you can just compose three morphs (Morph instances), a
big one with two smaller ones as children, and you get the same problem
in areas where the top morph overlaps a system window.

As I said, the problem is "simply" that #tryInvokeHalo: builds a stack
of potential targets at the clicked point, then for some reason that I
did not investigate discards the first one with the explicit comment
"existingHalo is first on the stack, not a target", while it *is* the
target here.

Best,

Stef

Chris Muller-3

Re: Bug in #tryInvokeHalo:

> > I just tried those steps. I can reproduce the problem when I click in

> > the *gray area* (e.g., parent-most morph) on the right side of the
> > halos, but when its in that same gray area on the left hand side, or
> > any other morph like the Help button, it works. Very strange. There
> > must be something unique about this particular "Same" game's Morph
> > structure or w.r.t. how it interacts with the new implementation of
> > #tryInvokeHalo:. This is the first Morph I've seen behave this way.
>
> No, the bug is very general - I just gave fast and easy steps to
> reproduce it. But you can just compose three morphs (Morph instances), a
> big one with two smaller ones as children, and you get the same problem
> in areas where the top morph overlaps a system window.

Strange, I just wrote the following code to try it:

| a b c| a := Morph new extent: 200@200.
b := Morph new color: Color green; position: 50@50.
c := Morph new color: Color yellow; position: 100@100.
a addMorphBack: b; addMorphBack: c; openInHand

and it worked. But then, I did it again, and it didn't.

> As I said, the problem is "simply" that #tryInvokeHalo: builds a stack
> of potential targets at the clicked point, then for some reason that I
> did not investigate discards the first one with the explicit comment
> "existingHalo is first on the stack, not a target", while it *is* the
> target here.

Ah! That's the clue we needed. I'm not exactly sure why the halo
morph wouldn't be the top one on the stack either, but this patch uses
a much more-resilient way to do that. Let me know if this works,
we'll commit it to trunk.

Best,
Chris

PasteUpMorph-tryInvokeHalo.st (3K) Download Attachment

marcel.taeumel

Re: Bug in #tryInvokeHalo:

Hi Chris,

that's the correct fix. :-)

Best,

Marcel

Am 25.04.2019 20:59:15 schrieb Chris Muller <[hidden email]>:
> > I just tried those steps. I can reproduce the problem when I click in
> > the *gray area* (e.g., parent-most morph) on the right side of the
> > halos, but when its in that same gray area on the left hand side, or
> > any other morph like the Help button, it works. Very strange. There
> > must be something unique about this particular "Same" game's Morph
> > structure or w.r.t. how it interacts with the new implementation of
> > #tryInvokeHalo:. This is the first Morph I've seen behave this way.
>
> No, the bug is very general - I just gave fast and easy steps to
> reproduce it. But you can just compose three morphs (Morph instances), a
> big one with two smaller ones as children, and you get the same problem
> in areas where the top morph overlaps a system window.

Strange, I just wrote the following code to try it:

| a b c| a := Morph new extent: 200@200.
b := Morph new color: Color green; position: 50@50.
c := Morph new color: Color yellow; position: 100@100.
a addMorphBack: b; addMorphBack: c; openInHand

and it worked. But then, I did it again, and it didn't.

> As I said, the problem is "simply" that #tryInvokeHalo: builds a stack
> of potential targets at the clicked point, then for some reason that I
> did not investigate discards the first one with the explicit comment
> "existingHalo is first on the stack, not a target", while it *is* the
> target here.

Ah! That's the clue we needed. I'm not exactly sure why the halo
morph wouldn't be the top one on the stack either, but this patch uses
a much more-resilient way to do that. Let me know if this works,
we'll commit it to trunk.

Best,
Chris