Strange behaviour when the touch event calls a scroller

Hello,

I recently upgraded my firmware to the cutting edge build espruino_2v24.125_banglejs2.zip to have the new Alarm update (long press to toggle alarm) and it introduced two bugs in my Elapsed Time app.

The first one: it is not possible anymore to add or substract two dates, as it returns NaN. The comparison operators also don't work anymore. The fix is easy : just add .getTime().

Example from the console:

>now.getTime() < target.getTime()
=true
>now < target
=false
>now - target
=NaN

The second one is more tricky. It seems like when the touch event calls a scroller, like this:

Bangle.on('touch', function (zone, e) {
  if (!inMenu && e.y > 24) {
    if (drawTimeout) clearTimeout(drawTimeout);
    showMainMenu();
    inMenu = true;
  }
});

Then the scroller is displayed, and immediately the menu row located where the touch event was is called. So it immediately goes into whatever menu item that was "under" the touch.
Not sure if I make much sense explaining it but here attached is a video that shows it: I tap the screen and the menu is displayed, and immediately the Date item is called because that's where my finger was.

Do you have any ideas on how to fix this?

Thanks

1 Attachment

• 847182654-VID_20241023_234036.mp4

Hm... I'm responsible for recent changes to showScroller and showMenu so now I feel a little guilty 😐

I'll take a closer look at your app tomorrow. But I wonder if just chucking a E.stopEventPropagation() in just before showMainMenu() would fix it for you?

Also what firmware did you have on before you installed this one?

Edit: just tried on 2v24.107 (which introduced the updated showScroller/Menu functions) without triggering the scroller/menu bug.

Edit2: can confirm I also get the scroller/menu bug when on 2v24.125.

No worries, it's no big deal :)
stopEventPropagation() worked like a charm!

Before upgrading to cutting edge I was using 2v23.

Edit: Spoke too soon. See next comment.

I narrowed it down.

The bug doesn't appear on fw 2v24.118: https://www.espruino.com/binaries/travis/f415a9f0b264ead525ea903f99b27441d7b1793f/

But does appear on the next compiled one, 2v24.120:
https://www.espruino.com/binaries/travis/10b6f5fe54de6697866a8b22d8d9948c05df5302/

One of the two commits after 2v24.118 changes the behavior so the menu's touch listener runs even though it wasn't registered at the time of the touch event. My guess is it has to do with the menu being called from within the touch listener that first act on the event.

So either:

https://github.com/espruino/Espruino/commit/445d331de3d9d035a3da86dc9583b899da2479bb (I suspect this one)

or

https://github.com/espruino/Espruino/commit/10b6f5fe54de6697866a8b22d8d9948c05df5302 .

Weird. Now I flashed 2v24.118 again and now I get the bug there as well... 😂

Edit:

Now I narrowed it down...

2v24.89 doesn't have the changed behavior:
https://www.espruino.com/binaries/travis/67da4057cb4cccdd15176cfdec4bc4341ac6058a/

... while 2v24.93 does:
https://www.espruino.com/binaries/travis/133933b82ae5a94b8bd8928ee0f7d8d15e962d47/

Between the two there were some refactors to the plumbing of registered event listeners.

It changes the behavior so the menu's touch listener runs even though it wasn't registered at the time of the touch event. My guess is it has to do with the menu being called from within the touch listener that first act on the event.

What tripped me up before was, the change/bug only seem to happen when elapsed_t was loaded in with a standard load call. If fastloaded to (e.g. going back from the launcher) it didn't happen.

The relevant commits:

• https://github.com/espruino/Espruino/commit/f63c15618700fd6b4f499844d91be16810402710
  
• https://github.com/espruino/Espruino/commit/d93c22352f396708b47fa00231592d3be13c805a
  
• https://github.com/espruino/Espruino/commit/b88a4d16b86b35b6779809bd1800a8d4434647ec
  
• https://github.com/espruino/Espruino/commit/133933b82ae5a94b8bd8928ee0f7d8d15e962d47
  

Thanks for the report @Pologram, and for tracking this down @Ganblejs! That's a bit of a pain. The change in event handling was to fix some errors other apps/people were having (which made things behave differently depending on whether there was a pre-existing event handler or not).

I was trying to make a very basic proof of concept for this, and:

var inMenu = false;
Bangle.on('touch', function (zone, e) {
  if (inMenu) return;
  inMenu = true;
  E.showMenu({
    A : ()=>print("A"),
    B : ()=>print("B"),
    C : ()=>print("C"),
    D : ()=>print("D"),
    E : ()=>print("E"),
    F : ()=>print("F"),
  });
});

or just

Bangle.on('touch', function (zone, e) {
  E.showMenu({
    A : ()=>print("A"),
    B : ()=>print("B"),
    C : ()=>print("C"),
    D : ()=>print("D"),
    E : ()=>print("E"),
    F : ()=>print("F"),
  });
});

don't seem to trigger it for me on latest firmwares. I even tried with an extra Bangle.on('touch', ()=>{}) just in case. I can't even seem to get your Elapsed Time app to fail the way you show it? Can anyone reproduce on 2v24.128 or later?

As an easy fix I think just changing showMainMenu(); to setTimeout(showMainMenu,0); should fix it, but it seems odd it can't be reproduced now?

Date

Interesting! (new Date()) + (new Date()) produces Thu Oct 24 2024 13:09:57 GMT+00001729775397865.07299804687

So on one call it's producing a string, and the other a number. The actual change at fault was a seemingly unrelated https://github.com/espruino/Espruino/commit/f09330e7baf42cb8f7fda2c8fec5fd9be11991df but I've managed to get a fix in now!

I can't even seem to get your Elapsed Time app to fail the way you show it? Can anyone reproduce on 2v24.128 or later?

I did get it to fail like that on 2v24.131. But now on 2v24.132 (and after reinstalling elapsed_t) it doesn't seem to happen for me anymore.

So probably it's fixed - but I'm not 100% sure.

Well, I take it back about the touch thing - it just happened to me in a settings menu and I'm not 100% sure why, and wasn't able to reproduce it.

I just tried putting another touch handler after and voila!

var inMenu = false;
E.showMessage("Tap");
Bangle.on('touch', function (zone, e) {
  if (inMenu) return;
  inMenu = true;
  E.showMenu({
    A : ()=>print("A"),
    B : ()=>print("B"),
    C : ()=>print("C"),
    D : ()=>print("D"),
    E : ()=>print("E"),
    F : ()=>print("F"),
  });
});
Bangle.on('touch', function() {
});

It seems 100% reproducible. So I think what was happening before was we completely re-wrote the list of event handlers when something was added/removed (and it was the list of handlers that got queued for execution). Now, we just modify that list so if you're iterating and you're not at the last entry when it executes it'll carry on.

So this needs fixing internally. I guess I'll go back to the old behaviour of making a whole new list of events for every modification - it's not so nice/efficient but I think it's more what people expect.

Ok, firmware is just fixed - @Pologram you could take those fixes out of your app now hopefully!

Thank you for the fast fix @Gordon!
I upgraded to 2v24.137 and both problems are now fixed!

So I think what was happening before was we completely re-wrote the list of event handlers when something was added/removed (and it was the list of handlers that got queued for execution). Now, we just modify that list so if you're iterating and you're not at the last entry when it executes it'll carry on.

I just thought of a possible way around this - but I don't know if it's better or worse performance-wise. And there is probably some other drawbacks as well.

But - if X.on prepended listeners instead of appending them, maybe we could have the best of both worlds? The newly registered touch listener in the elapsed_t case would instead sit on top and therefore not run from the current event.

If this was to be adopted I suggest:

• X.on starts acting exactly like X.prependListener,
  
• X.prependListener stays the same,
  
• a new X.appendListener that would work like X.on worked before.
  
  

A strong argument against this is it breaks spec compliance - if I understand correctly.

Thanks - that is an interesting thought! It does make some sense, but I think it would break other things - for example don't you have a handler in boot code that changes your touchscreen coordinates? That'd no longer be possible if everything else put the handler in front of it by default...

I think the current solution is probably safe/sensible enough for now.

for example don't you have a handler in boot code that changes your touchscreen coordinates?

I don't think that case should hold us back. It'd be easy for me to handle that I believe, I'd just make sure to reregister that drag listener in particular every time another listener was prepended/X.on-ed.

But I see your point, there are certainly some more apps/bootcode that expects to remain more or less at the fore of listeners and it would maybe become a headache.

I think the current solution is probably safe/sensible enough for now.

Agreed.