search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Many commands (including save) apply to wrong file just after using Quick Open #2812

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by njx Thursday Feb 28, 2013 at 01:25 GMT Originally opened as https://github.com/adobe/brackets/issues/2985

I think this set of steps reproduces the problem reliably.

  1. Open two JS files in the working set (and view both of them).
  2. Switch to the first file.
  3. Type something into the file that would cause a JSLint error (e.g. add "x" at the beginning of the file) and save it so the JSLint errors show up.
  4. Switch to the second file.
  5. Type something into the file that would cause a JSLint error, but don't save it yet.
  6. Using Quick Open, switch back to the first file (the one you modified in step 3).
  7. Click back on the second file in the working set.
  8. Try to save it.

Result: Saving doesn't work--Cmd-S or File > Save don't do anything (dirty dot is still visible and file isn't changed on disk). However, if you try to close the file and choose to save it, it does work; or if you switch to another file and switch back, that fixes it.

Also, a number of times when this happens, I've noticed that JS code hinting is in a weird state, where it thinks I'm in the middle of typing a string when I'm not, so it shows me string hints. I'm not sure how that would be related, but they happen together too often for this to be a coincidence, I think.

Finally, note that if you don't use Quick Open to switch to the first file (as in step 6), the problem doesn't seem to occur.

UPDATE: Possibly also related...I've noticed that in a couple of cases, the string "use strict" seemed to be randomly added to the very beginning of files I might not have actually touched. That reinforces my thinking that JS code hinting is somehow getting messed up, because that string might come from the string hints that come from JS code hinting.

core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 01:32 GMT

BTW, when this happens I don't see any errors in the inspector.

core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 02:04 GMT

Adding to sprint 21--need to figure out who can look into this.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 02:19 GMT

@njx If you hit it again, can you put a breakpoint in DocumentCommandHandlers.handleFileSave() and see if it's even getting called? And if so, what does it get as activeEditor? (seems like that getting out of sync could possibly account for both Save and code hints breaking).

core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 02:32 GMT

I just updated the steps--I think the recipe above is reliable.

Weirdly, I can't seem to ever hit a breakpoint in handleFileSave() or doSave() even when save is working, which doesn't make sense. (I just relaunched Brackets, and I can hit other breakpoints.) Not sure what's going on. I don't have time to look further into it right at the moment though.

@peterflynn -- can you check quickly if you can repro this using my steps above? Since I'm going to be out tomorrow I want to make sure at least one other person can repro (even if you don't end up fixing the bug).

core-ai-bot commented 3 years ago

Comment by RaymondLim Thursday Feb 28, 2013 at 03:05 GMT

I confirmed that JS code hinting extension is not the culprit; I remove the entire JS Code hints extension and I still can reproduce the issue with your updated steps. And also it seems to be affecting Save command only and not all keyboard shortcuts; I still can undo with Ctrl+Z when Save command is failing.

core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 03:40 GMT

Interesting. It does seem, however, that the JS hint extension is somehow affected.@peterflynn suggested that there might be some problem with the active editor being set incorrectly, which would explain some of these mysterious issues.

core-ai-bot commented 3 years ago

Comment by jbalsas Thursday Feb 28, 2013 at 04:13 GMT

@njx I've been looking into this, and I think you can reproduce it with an easier recipe:

  1. Open two JS files in the working set (and view both of them).
  2. Switch to the second of them
  3. Using Quick Open, switch back to the first file.
  4. Click back again on the second file in the working set.
  5. Write anything and try to save it.

I've checked and it is indeed an issue with the active editor being a wrong one (the dirtyflag is then read from the clean editor), and it also seems somehow a race condition (If I set a breakpoint in handleFileSave or doSave, it does work for me all the time).

I'm not that familiar with this code, so I still don't have a fix, but maybe this'll help narrow it down.

core-ai-bot commented 3 years ago

Comment by jasonsanjose Thursday Feb 28, 2013 at 04:13 GMT

Assigned to me

core-ai-bot commented 3 years ago

Comment by jasonsanjose Thursday Feb 28, 2013 at 04:18 GMT

Confirmed using@jbalsas steps.

core-ai-bot commented 3 years ago

Comment by jasonsanjose Thursday Feb 28, 2013 at 04:33 GMT

I'm seeing runtime errors in the console

Uncaught TypeError: Cannot read property 'forceSelect' of undefined
/Applications/Brackets%20Sprint%2021.app/Contents/dev/src/thirdparty/smart-auto-complete/jquery.smart_autocomplete.js:348
core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 05:11 GMT

I don't think I saw those--maybe something unrelated? It's also possible my dev tools are somehow hosed though, since I was having the breakpoint trouble mentioned earlier.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 07:28 GMT

I've narrowed it down a tad more. When you do the final editor switch -- going to the file with unsaved changes -- Editor's focus listener fires for the other document instead. Clearly this is incorrect, because when you type or hit the arrow keys it goes to the editor with unsaved changes, not that other one.

So it seems possible this is a CM bug. Could the extra refreshes caused by the JSLint stuff jumping around be throwing off its own focus listeners somehow??

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 08:57 GMT

The preceding QuickOpen seems to get CM for the second file (w/ unsaved changes) in a state where it thinks it has focus even though it doesn't. This causes it to not send a focus event when it gains focus later.

The trouble with Quick Open looks to be how the search bar closes: it's triggered by losing focus, and it loses focus due to switching to the other file. So while we're already in the midst of switching to over the first file, the search bar loses focus, closes and asks to restore focus to the "active" editor, which is still the second file (because the focus change gets to the search bar before it gets to EditorManager). So the first file gets focus, which causes the second file to be given focus, and then the first file gets focus again (seemingly because there is some redundant logic in CodeMirror.focus()). I haven't yet pieced together exactly how that causes CodeMirror's state.focused to get out of sync, but that's the high-level of it.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 09:01 GMT

Fixing this seems tricky. The timing of Quick Open teardown is extremely fragile, as we know from previous bugs. The timing of activeEditorChange is also fairly delicate, as we know from the really nasty bugs back in October. But I wonder if we could make an exception for the case where currentEditor is changing, essentially changing activeEditor ahead of the focus even in that case... Or perhaps the Quick Open select handler could be smarter about closing its own search bar before initiating the document change...

OTOH, it seems like this bug must also exist in Sprint 20 so that may make it a tad less urgent. OTOOH, it may well be CMv3-dependent, meaning it's still a fairly new regression...

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 09:12 GMT

This may also be what's causing #2951 -- if so, then the issue is pretty old since that bug has been occurring at least since December (presumably since ModalBar was introduced).

core-ai-bot commented 3 years ago

Comment by njx Thursday Feb 28, 2013 at 15:48 GMT

I don't feel like I've ever seen this before, and I'm seeing it all the time now, so it does feel like it might have been injected or exacerbated by the cmv3 switch. Might be worth git bisecting to see if that's the case (remembering to do a submodule update when you hit a commit that changes the CM SHA).

core-ai-bot commented 3 years ago

Comment by RaymondLim Thursday Feb 28, 2013 at 16:05 GMT

Actually, I can reproduce it in Sprint 19. So it may not be related to cmv3.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 18:06 GMT

Yup, I can confirm -- repros in Sprint 19 & 20 with the same steps. I'm guessing Sprint 18 too since that's when ModalBar was originally introduced.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 19:49 GMT

Lowering to Medium since it's been around for several sprints with no reports until now.

core-ai-bot commented 3 years ago

Comment by jbalsas Thursday Feb 28, 2013 at 20:11 GMT

Note that this also affects any other operation that would use the activeEditor. For example, a simple search runs on the first editor when the second one is visible. Nothing is highlighted in the visible document, but pressing repeatedly cmd+g advances the hit on the first one and updates the status bar cursor information.

@peterflynn Maybe you could consider renaming the title as the bug is not bound to just saving files and files with jslint errors?

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 20:13 GMT

Sure -- I've clarified the title

core-ai-bot commented 3 years ago

Comment by RaymondLim Thursday Feb 28, 2013 at 20:24 GMT

I just confirmed that this issue is introduced in sprint 19; can't reproduce it in sprint 18 and earlier versions.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 28, 2013 at 22:49 GMT

Interesting, thanks Raymond. That makes me suspicious that the subtle timing changed in #2548 may be causing this. Although how, I don't know -- the problem seems to be that hitting Enter opens the new file before it triggers closing the modal bar, and although Enter is processed slightly later after #2548 that ordering was still true before it too.

core-ai-bot commented 3 years ago

Comment by pthiess Friday Mar 01, 2013 at 20:59 GMT

Reviewed, agreed that we want this fixed asap.

core-ai-bot commented 3 years ago

Comment by peterflynn Saturday Mar 02, 2013 at 01:56 GMT

So there are a couple potential fixes I've come up with so far... none risk-free, though:

core-ai-bot commented 3 years ago

Comment by njx Saturday Mar 02, 2013 at 02:22 GMT

Urgh. I vaguely recall that the original CM search "dialog" (which ModalBar replaced) did something where it did a setTimeout() in order to avoid a similar issue with blur and focus. I got rid of that in my version because it seemed unnecessary, but maybe it was trying to fix something like this. I don't understand the problem well enough to know though.