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

[CLOSED] Fix for documents refCount issue #10627

Open
core-ai-bot opened this issue Aug 30, 2021 · 13 comments
Open

[CLOSED] Fix for documents refCount issue #10627

core-ai-bot opened this issue Aug 30, 2021 · 13 comments

Comments

@core-ai-bot
Copy link
Member

Issue by swmitra
Thursday May 05, 2016 at 07:15 GMT
Originally opened as adobe/brackets#12405


This PR provides a fix for the document refCount issue reported as part of #12396. This resolves the URL Code hints unit test failures barring the one which is to deal with '@import'.

CC@MarcelGerber@petetnt@nethip@abose for review.


swmitra included the following code: https://github.com/adobe/brackets/pull/12405/commits

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Thursday May 12, 2016 at 19:03 GMT


I was trying the STR in adobe/brackets#12396, but with this PR when i click on the filename of the inline editor it doesn't open the file.
The cursor is moved at the correct line but in the current file.

@core-ai-bot
Copy link
Member Author

Comment by nethip
Friday May 13, 2016 at 05:59 GMT


@swmitra Could you address the query posted by@ficristo ?

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Friday May 13, 2016 at 13:04 GMT


@ficristo Thanks a lot for actually trying the fix 👍

I need one clarification regarding the behavior you have mentioned. I am guessing that the inline editor and the host editor in your case is pointing to the same document. If that's the case then, even without this fix the behavior is same in master.

Can you please confirm?

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Friday May 13, 2016 at 13:14 GMT


@swmitra I've opened the SpecRunnerUtils.js and put the cursor on 406 (this on current master).
Pressing Ctrl-E it opens the inline editor pointing to EditorManager line 156.
If I click on the name it move the cursor on line 156 but of SpecRunnerUtils.
It should instead open EditorManager at lime 156.
I've tryed this after reload without extensions.

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Friday May 13, 2016 at 13:21 GMT


Thanks@ficristo. It's clear now, I will fix it tonight.

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Tuesday May 17, 2016 at 11:35 GMT


@ficristo@MarcelGerber Can you please try with the recent changes?
Thanks in advance 😄

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Tuesday May 17, 2016 at 11:51 GMT


Now when I click on the inline editor I have this error:
/src/editor/EditorManager.js found in pane working set but pane.addView() has not been called for the view created for it

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Wednesday May 18, 2016 at 10:48 GMT


@ficristo It was getting caused by a missing check in _showEditor(). I had put it back now. URL Code hint test cases are passing as well. Can you please check it once more (hopefully for the last time 😄 ).
Thanks again for helping us out in verifying the fix 👍

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Wednesday May 18, 2016 at 12:44 GMT


There is still an error on Url Code Hints, not sure if is related to this or not.
I couldn't reproduce the exact steps mentioned by@MarcelGerber in adobe/brackets#12396: I have a different _refcount number.
I tested what I could and it seems good; someone with better knowledge should look at it too.

@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Thursday May 19, 2016 at 12:54 GMT


@ficristo This one test failure is fixed on master already, so with latest master merged in, all test pass 👍
I've followed my repro steps and even though I didn't get the same numbers in the first place, as a result, I had only one document left open. Thumbs up for that! Good work,@swmitra!

@core-ai-bot
Copy link
Member Author

Comment by nethip
Thursday May 19, 2016 at 13:08 GMT


@swmitra I have taken liberty in changing editorCount++ to `++editorCount.

@core-ai-bot
Copy link
Member Author

Comment by nethip
Thursday May 19, 2016 at 13:10 GMT


Since travis is not able to run because of API limitation, I have manually run grunt build and the build went fine. So merging this.

Thanks@swmitra 👍

And thanks to@MarcelGerber@ficristo for helping this PR come to light.

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Thursday May 19, 2016 at 13:13 GMT


Thanks a lot@nethip@MarcelGerber@ficristo for helping us resolving the issue 👍

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

1 participant