Mirror input thrown off if mouse leaves upper iframe.

[KatieWoe]

For https://github.com/phetsims/QA/issues/636. May be related to recent iframe changes. I think I may have also seen something like this in the record wrapper where events didn't match what I did for a while, but I'm not sure if this could cause that. I'll keep trying to reproduce that one. For this issue, if you zoom in and use the mouse to start panning around, and your mouse leaves the upper iframe and lets go, then coming back will stop panning in the upper frame but not the lower pane. I suspect something similar would happen for other draggable objects, but this sim has few of them.

[KatieWoe]

@pixelzoom let me know if you want me to move this to a different repo.

[KatieWoe]

I did some more testing and it seems like this does happen in the recording wrapper as well.

[pixelzoom]

"May be related to recent iframe changes" likely refers to https://github.com/phetsims/scenery/issues/1186 .

This doesn't seem at all specific to the sim, and it's outside of my wheelhouse. Assigning to the PhET-iO team to investigate. If this is indeed a general problem, please create a general GitHub issue in the appropriate repo, and link it here.

[zepumph]

@jonathanolson what action/event from input is not being properly captured by the downstream system? Presumably it is something that is interrupting the pointer in the upstream. Is there anything more that lostPointerCaptureAction or gotPointerCaptureAction? If not perhaps it is that the event doesn't look as we suspect. I'll investigate.

[zepumph]

I didn't see any *PointerCapture events with ?sceneryLog=Input (which only applies to the top screen) when I reproduced this case during spots when I thought I would. I guess I don't really understand these two new events, and I would recommend @jonathanolson instead take the lead on the debug.

Index: mirror-inputs/mirrorInputs.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/mirror-inputs/mirrorInputs.js b/mirror-inputs/mirrorInputs.js
--- a/mirror-inputs/mirrorInputs.js (revision 32f18430faeeb05249cad988a30b922684e13f9b)
+++ b/mirror-inputs/mirrorInputs.js (date 1617672693807)
@@ -37,7 +37,7 @@
     sourceQueryParametersTail = QueryStringMachine.removeKeyValuePair( sourceQueryParametersTail, 'postMessageOnError' );
   }

-  sourceFrame.src = QueryStringMachine.appendQueryStringArray( simURL, [ 'frameTitle=source', sourceQueryParametersTail ] );
+  sourceFrame.src = QueryStringMachine.appendQueryStringArray( simURL, [ 'frameTitle=source&phetioConsoleLog=json', sourceQueryParametersTail ] );

   const sourceClient = new phetio.Client( sourceFrame );
   sourceClient.addErrorListener( error => { throw error;} );

It could be helpful to use this patch to see the upstream phet-io events being created. This in correlations with ?sceneryLog=Input would be my recommendation on solving this.

[jonathanolson]

I'm reproducing in master. With ?sceneryLog=Input, I am seeing lost pointer capture:

[Input] lostPointerCapture('1',26881.969999987632 lostpointercapture);

Investigating

[jonathanolson]

I've identified the cause. The code that sets the pointer ID on the mouse (this.mouse.id = id;) is in pointerDown, which is outside of the actions, and isn't run on playback.

It seems the playback system is based on higher-level abstractions than are probably desired longer-term for playback (we'd really want to record things at the event level for accurate playback, e.g. there are three different events that can trigger a mouseDown action, but we only record the mouseDown). NOTE: This would be less than ideal presumably for a usable event stream.

I'll look into less invasive workarounds for now.

[jonathanolson]

The best change from my perspective would be adding an optional id parameter (number or null) to mouseDownAction (and mouseDown()), which would be used in the code to set the mouse's ID. mouseUp would also need to be modified to clear this ID.

This sounds like it would break backward compatibility to event recording. @zepumph thoughts?

@pixelzoom, do you know the priority for this work?

[pixelzoom]

@pixelzoom, do you know the priority for this work?

You'll have to ask @kathy-phet and @amanda-phet whether this is a blocking issue.

If this is a blocking issue, priority is high. The goal is to complete dev test phetsims/QA#636 as quickly as possible (in the next day or two), and then immediately create a release branch (before changes sneak into master).

Since this was discovered in dev testing, there is no release branch that needs to be patched. But if this is deemed a blocking issue, it will need to be resolved in master before the release branch can be created.

[jonathanolson]

Hopped on a call with @zepumph and we determined that the ideal long-term solution was adding the optional ID to mouseDown/mouseDownAction. There is a whole host of low level code that should NOT run in playback, so I believe the actions ARE at the correct abstraction level.

Fixes are above. @KatieWoe can you verify in master?

[KatieWoe]

It looks like this is solved on master

[jonathanolson]

Perfect! Closing.