Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Playback missing characters #151

Closed
jlfwong opened this Issue May 5, 2012 · 5 comments

Comments

Projects
None yet
2 participants
Contributor

jlfwong commented May 5, 2012

In http://khan-labs-test.khan-academy.appspot.com/labs/code/763968974, at around the (-3:26) mark, we wrote in 180, and only 80 is entered into the editor.

This breaks the rest of the demo.

Owner

jeresig commented May 7, 2012

I'm struggling to figure out /how/ #151 could've happened. I've been looking at the logs and it looks like the number slider was adjusted -- and then backspace is hit twice and then 8 and 0 are entered. This makes some sense as after the slider was done the number "120" was entered -- BUT the selection was covering the entire number 120, thus when two backspaces occur it deletes the entire 120 and the space before it, breaking everything. SO, the question now becomes, that based upon this log SOMEHOW the selection on the number 120 was remove, the cursor was after the number, two backspaces remove the 0 and 2, and everything from thereon would work fine.

I've tried every way that I can think of to clear a selection from a number -- but all the ways that I can think of are logged to the system. So we need to find some way of clearing the selection that somehow isn't logged - and then find a way to log it. Any insight here would be appreciated.

For example, this is a good sequence (you guys tweaked the slider, changed the selection, hit backspace twice -- it worked:
http://ejohn.org/files/screen/2012-05-07_1101.png

And here is the bad one, note the lack of change in selection:
http://ejohn.org/files/screen/2012-05-07_1102.png

Owner

jeresig commented May 7, 2012

If you want to help me research this, you can use these functions:

Record.dump = function() {
    if ( Record.commands ) {
        return Record.commands.map(function( item ) {
            var ret = [];

            for ( var prop in item ) {
                if ( prop !== "time" ) {
                    ret.push( prop + ":" + item[prop] );
                }
            }

            return ret.join();
        }).join();
    }
}

Record.log = function(a) { console.log( JSON.stringify(a) ); };

dump those into the console then run:

Record.record();

Try adjusting the selection to see if you can ever clear the selection without a corresponding log occurring.

Contributor

jlfwong commented May 7, 2012

I can try to look into it later - what's the long term plan here? Key/mouse playback doesn't seem to be a particularly robust solution, and seems incredibly difficult to debug.

I understand that it is by far the simplest implementation, but it's not without flaws.

Two use cases I thought of that are likely to break it:

  1. Differences between OS's, e.g. command-A compared to ctrl-A - won't the playback break?
  2. Copy and paste - how is this going to work?

In the long term, it seems like it'll be necessary to record textual/selection changes, not just keystrokes.

Owner

jeresig commented May 7, 2012

I'm not actually logging keystrokes or mouse movements - I'm logging the commands coming directly from the editor. Thus I'm logging keys and commands. This means that there is a complete abstraction in-between what is happening and what is being recorded (command-A vs. ctrl-A is completely taken care of, for example). Additionally all the other commands like copy/cut/paste are also being logged as are undo/redo and stuff like that. The code is relatively straight-forward:
https://github.com/Khan/canvas-editor/blob/master/js/editor.js#L56-151

All things considered it seems to be quite robust - the two issues that've been hit thus far are:

  • Being able to change selection in the editor before recording begins (which is what happened with an earlier recording of yours). That's easy enough to prevent.
  • And this, current, mystery bug.
Owner

jeresig commented May 7, 2012

If we think long-term debug-ability could be an issue I could easily add in full logging capabilities - one that captures every single thing that the editor spews out. It'll be a ton of data but we could always sift through and and correlate what we see there with what's in the simplified log that's being sent to the server (it's possibly that I have an error in my logic that's excluding some specific selection command that's causing the issue that we see here).

@jeresig jeresig closed this in 419ff54 May 7, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment