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

Calls won't update if not visible #63

Open
Drazzzt opened this issue Jul 17, 2014 · 4 comments
Open

Calls won't update if not visible #63

Drazzzt opened this issue Jul 17, 2014 · 4 comments

Comments

@Drazzzt
Copy link

Drazzzt commented Jul 17, 2014

Hi,
the calls counter won't update if not visible. Only if executed when visible they will update again (and then e.g. jump from 10 to 12).

@Drazzzt
Copy link
Author

Drazzzt commented Jul 17, 2014

(this is only the case if the calls are not selected (not blue))

@alltom
Copy link
Member

alltom commented Jul 18, 2014

I thought I'd fixed this really early on! Let me check it out and see what's regressed. Thanks for the report.

@alltom
Copy link
Member

alltom commented Jul 18, 2014

You're using Brackets 0.41, right?

@alltom
Copy link
Member

alltom commented Jul 18, 2014

I think I can reproduce the problem.

  1. Put this in foobar.js:

    setTimeout(foobar, 3000);
    
    // ... lots of blank lines ...
    
    function foobar() {}
  2. Open foobar.js in Brackets with Theseus.

  3. Run node-theseus foobar.js.

  4. Right away, scroll to the bottom in Brackets to see "0 calls" next to foobar(). Then immediately scroll up to the top.

  5. Wait 3 seconds.

  6. Scroll down to the definition of foobar().

Expected: it says "1 call" next to foobar().
Actual: it says "0 calls" next to foobar().

If you skip step 4, it says "1 call" as expected.

Diagnosis

CodeMirror removes those call count pills from the DOM for efficiency as you scroll around. When Theseus does a batch update of all the hit counts, it minimizes DOM access by only updating counts for the functions that have been called since the last update. But it also checks for pills that have been added back by CodeMirror to give them their most up-to-date value, even if the corresponding functions weren't called in the current batch.

It has to do that because it used to be that, when CodeMirror re-added a pill, it inserted the HTML that was originally used to create it, so any changes (such as updating the number and changing the style) were reversed. Theseus took advantage of that by adding an "uninitialized" class to that initial HTML, so it could find re-added pills by looking for nodes with the "uninitialized" class.

My theory is that the latest CodeMirror actually adds the original DOM node back (with all subsequent changes) instead of creating a new node with the original HTML. That explains why it works if you don't scroll far enough for the pill to be on the screen and have its "uninitialized" flag cleared, but doesn't work if you do scroll down.

If the theory is correct, then instead of looking up DOM nodes by ID to update them (which I only did because CodeMirror would destroy and recreate the nodes whenever it felt like it), Theseus can retain a pointer to the DOM nodes for the pills and update them directly, always.

Time to experiment!

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

No branches or pull requests

2 participants