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] Control+click to show context menu removes selection (previously: menu doesn't stay open) #1102

Open
core-ai-bot opened this issue Aug 29, 2021 · 16 comments

Comments

@core-ai-bot
Copy link
Member

Issue by pthiess
Friday Jun 22, 2012 at 05:44 GMT
Originally opened as adobe/brackets#1111


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

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Friday Jun 22, 2012 at 17:18 GMT


UTR for me on 10.7.

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Friday Jun 22, 2012 at 17:58 GMT


Reviewed - take for Sprint 10

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Friday Jun 22, 2012 at 17:58 GMT


@redmunds

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Jun 22, 2012 at 22:21 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Saturday Jun 23, 2012 at 00:14 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Sunday Jun 24, 2012 at 02:19 GMT


Verified fix is sufficient, closing.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Sunday Jun 24, 2012 at 15:17 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Monday Jun 25, 2012 at 03:38 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Jun 26, 2012 at 04:40 GMT


Updated title to clarify the symptoms

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Jun 26, 2012 at 04:44 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Tuesday Jun 26, 2012 at 04:58 GMT


Thanks for updating the bug, Peter :)

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Jun 26, 2012 at 18:11 GMT


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 :-)

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Jun 26, 2012 at 18:21 GMT


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).

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Jul 10, 2012 at 15:44 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Monday Jul 16, 2012 at 17:14 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by pthiess
Tuesday Jul 17, 2012 at 23:53 GMT


Verified on Mac 10.7.4 - closing fixed.

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