[CLOSED] Working set context menu pops closed when right-clicking file that hasn't been opened yet

[core-ai-bot]

Issue by peterflynn
Monday Oct 22, 2012 at 05:34 GMT
Originally opened as adobe/brackets#1910

---

1. Place several files in the working set
2. Restart Brackets
3. Right-click a working set entry that hasn't been viewed in the editor yet

Result:
Context menu appears briefly, then closes again.

Expected:
Context menu stays open.

Workaround:
Right-click it again.

[core-ai-bot]

Comment by peterflynn
Monday Oct 22, 2012 at 06:10 GMT

---

This also happens when right-clicking files in the project tree (if they are not open in an editor, i.e. not in the working set and not currently selected).

[core-ai-bot]

Comment by RaymondLim
Monday Oct 22, 2012 at 16:35 GMT

---

I can't reproduce it on Win7. So must be a mac only issue.

[core-ai-bot]

Comment by peterflynn
Monday Oct 22, 2012 at 17:14 GMT

---

I can repro on WinXP. I'm guessing it depends on how quickly the file is opened (try with a larger file, perhaps, if you're on a faster system?).

[core-ai-bot]

Comment by pthiess
Monday Oct 22, 2012 at 18:36 GMT

---

Reviewed might be related to the popup management bug #1381. In Trello Card 643, closing.

[core-ai-bot]

Comment by pthiess
Monday Oct 22, 2012 at 18:39 GMT

---

Re-opend since we want to keep issues visible if not ECR.

[core-ai-bot]

Comment by peterflynn
Wednesday Jan 09, 2013 at 19:19 GMT

---

So here's how the bug plays out:

1. Right mousedown causes WorkingSetView to ask to open the document (via event.which !== 1 check in _reorderListItem() calling drop())
2. We begin loading the document from disk
3. Right mouseup followed by contextmenu event. Context menu is popped open.
4. Done loading the document; currentDocumentChange fires
5. In response, ProjectManager._redraw() is run three times. Evidently in order to "reposition the selection triangle," it forcibly triggers a scroll event on the file tree.
6. Whenever scroll occurs on the file tree, ProjectManager calls Menus.closeAll() -- snapping the context menu shut again.

In scenarios where the disk is fast or the file was already loaded, currentDocumentChange occurs before mouseup and thus the context menu opens after closeAll().

Seems like ths ideal fix is, don't fake a scroll event to trigger updating the triangle position. Barring that, we could detect fake scroll events and only call closeAll() when a real user-driven scroll has occurred.

[core-ai-bot]

Comment by peterflynn
Wednesday Jan 09, 2013 at 19:22 GMT

---

Actually, there's a subtler bug here too: in the interval where the context menu has been opened but the document isn't yet loaded, the tree selection still lies on the old item. This means that if you manage to click a context menu command during that time, it will actually apply to the wrong file.

Probably near-impossible with our current local-disk APIs, but in browser with a network backend this could be a real problem. Along with many other similar bugs we have lurking -- so we should probably just spin that off as a separate issue.

[core-ai-bot]

Comment by njx
Wednesday Jun 19, 2013 at 16:07 GMT

---

Bumping to medium priority since a number of people have run into it. I think we have too many bugs open for sprint 27 already, but should consider it for the next sprint.

[core-ai-bot]

Comment by lkcampbell
Sunday Jun 23, 2013 at 03:44 GMT

---

I did a little bit of research on this bug today.

Just as a test, I commented out the fake scroll event in ProjectManager._redraw(). It does get rid of those three Menus.closeAll() calls that come from the triggered fake scroll events, as@peterflynn said, but it still doesn't fix the bug because there is another Menus.closeAll() call that occurs after those three calls.

The last call is coming from Editor.js. It is an anonymous function that fires on the CodeMirror scroll event. I assume the Editor is scrolling somehow as the new document is loaded. So, this needs to be addressed as well.

[core-ai-bot]

Comment by njx
Sunday Jun 23, 2013 at 06:17 GMT

---

The "always close popups on scroll" behavior also causes #3391. I wonder if we need a smarter heuristic here, but I'm not sure what it would be. At the very least, scrolls in one area shouldn't affect popups that are associated with completely different areas of the UI. But even within a given area the existing behavior is problematic because the scrolls are happening asynchronously after the popup appears.

[core-ai-bot]

Comment by lkcampbell
Thursday Aug 29, 2013 at 05:03 GMT

---

@njx and@peterflynn, I think I will spend some more time researching this bug. That is, unless you guys were planning to make it a Sprint 31 bug. Otherwise, I am going to work on it this long weekend.

If you have any ideas or suggestions for possible solutions, I would welcome them as well.

[core-ai-bot]

Comment by njx
Thursday Aug 29, 2013 at 17:46 GMT

---

@lkcampbell Thanks for looking at this! I don't have any ideas off the top of my head...let us know what you come up with.

[core-ai-bot]

Comment by lkcampbell
Friday Aug 30, 2013 at 03:37 GMT

---

@njx and@peterflynn, I tried to keep the fix as simple as possible. Instead of triggering a scroll event, which is much too broad just to update the triangle selector, I made a new, specific event, selectionRedraw, that only updates the selector. That way, we don't get the undesirable extra effects of a triggered scroll event.

[core-ai-bot]

Comment by lkcampbell
Friday Aug 30, 2013 at 16:06 GMT

---

Also, the problem I was seeing, where the anonymous function in Editor.js is sending a scroll event that closes all menus, seems to occur much less frequently now. I have a difficult time reproducing it whereas, when I originally documented that observation, it happened almost every time I opened a new document.

I think the problem of a broad-based closing of popups needs to still be address, as@njx mentioned, but since it doesn't appear to be causing any major problems in this issue any more, I will try to address it in a fix for issue #3391.

[core-ai-bot]

Comment by jasonsanjose
Thursday Sep 05, 2013 at 22:51 GMT

---

FBNC@peterflynn

[core-ai-bot]

Comment by lkcampbell
Wednesday Oct 02, 2013 at 22:02 GMT

---

@peterflynn, reminder FBNC.

[core-ai-bot]

Comment by peterflynn
Saturday Nov 16, 2013 at 01:29 GMT

---

Hmm, I got one case where the menu simply didn't appear at all, but I haven't been able to reproduce it. Seems fixed in general. I'll file a new bug if I can find reliable steps for that one glitch.

[core-ai-bot]

Comment by lkcampbell
Saturday Nov 16, 2013 at 04:33 GMT

---

@peterflynn, if it is at all helpful for discovering the repro steps, this disappearing menu problem might be due to a Scroll event listener that is set up in Editor.prototype._installEditorListeners. I have not yet been able to figure out why the Editor is firing off a scroll message when changing from one file to another but I have been able to trace the problem to a scroll from an anonymous function in Editor.