Control+click to show context menu removes selection (previously: menu doesn't stay open)

[pthiess]

Not sure if this is expected to work yet, according to https://trello.com/c/Um2Nlhh9

Steps to reproduce (Mac10.6, 10.7)
1.Open a HTML file in Brackets
2. Place the cursor in an item which allows quick edit
3. Use Cmd + e to verify that Quick Edit actually works
4. Close Inline editor
5. Place cursor in same position again, but now use control+click (doesn't repro for right-click or two-finger tap)
6. ((old)) Must click slow (doesn't repro for fast click)

Observed behavior
Context menu appears, but no text is selected. If you had a text selection before right-clicking, the selection is lost.
((old)) Context Menu flickers and goes away, user is not able to trigger command

Expected behavior
Should work like normal right-click or two-finger tap: if no text was selected before clicking, the word at the click location should become selected; if there was a selection, it should be preserved.
((old)) Selection is made and context menu is shown permanently to enable selection of a command

[jasonsanjose]

UTR for me on 10.7.

[pthiess]

Reviewed - take for Sprint 10

[pthiess]

@redmunds

[redmunds]

Also a CodeMirror change: https://github.com/adobe/CodeMirror2/pull/66

[redmunds]

FBNC back to @pthiess

Peter, please verify that you think this fix is sufficient for Sprint 10. We think this needs a better fix which is too risky for now, so leave the issue open so we can revisit this issue.

[pthiess]

Verified fix is sufficient, closing.

[redmunds]

Leaving this issue open for a better fix. Removed Sprint 10 tag.

[pthiess]

Yeah, makes sense saw your pull request, thanks Randy.

[peterflynn]

Updated title to clarify the symptoms

[peterflynn]

Updated description to describe the current state after the partial fix / mitigation patch landed. Also, nominated for Sprint 11 since this is getting hit in the wild already.

[pthiess]

Thanks for updating the bug, Peter :)

[peterflynn]

Just to recap the technical issues here... The underlying problem here is that the events sent by a ctrl+click look a lot more like a left click than a right click, even though it should get interpreted as the latter. Chromium dispatches all the mouse events with which = 1 (left button), and it dispatches a click event at the end (which never happens if you right click with a real mouse or if you two-finger tap).

So the right-click events get interpreted as a left click by several listeners. Two in particular: the bootstrap-dropdown.js click listener (which hides the popup prematurely just moments after the mousedown showed it), and CodeMirror's mousedown listener (which treats it as a regular left click, blowing away any selection with an insertion cursor just before the contextmenu event will open the menu).

The click bug is easier to fix because there's a trick for making click events disappear: if the mousedown and mouseup occur over different DOM nodes, boom, no click event follows the mouseup. The popup menu will have this effect so long as it appears underneath the mouse position. In most cases, it already did, but we had one special case that positioned it lower (which was odd anyway since it was inconsistent). Pull #1131 removes that case, ensuring we never get a click event. (The earlier pull #1126 patched bootstrap-dropdown.js directly).

The remaining bug, with selection getting dropped, seems like it should require a CodeMirror patch (pull adobe/CodeMirror2#66). It even occurs with CEF's native context menu if all our context menu code is turned off. Of course, in an ideal world the browser would just send less confusing events :-)

[peterflynn]

Also note: the CodeMirror bug repros in a vanilla browser testbed (like theme.html) as well, as long as you've passed the option dragDrop: false.

(With dragDrop: true, CodeMirror doesn't clear selection until mouseup, which avoids the bug in a similar manner to #1131: the menu pops up before mouseup, so as long as it's under the mouse it gets the event instead of CodeMirror. Both the native menu and ours are positioned that way).

[redmunds]

This one is waiting for @peterflynn to merge codemirror changes, so re-assigning.

[peterflynn]

I verified this is fixed by the latest CodeMirror merge: adobe/CodeMirror2#67. FBNC @pthiess.

[pthiess]

Verified on Mac 10.7.4 - closing fixed.