Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 370584

Summary:	[Firefox] Edit menu items in context menus
Product:	[ECD] Orion	Reporter:	Mihai Sucan <mihai.sucan>
Component:	Editor	Assignee:	Mihai Sucan <mihai.sucan>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P2	CC:	eclipse.felipe, mihai.sucan, Silenio_Quarti
Version:	unspecified
Target Milestone:	0.4 RC1
Hardware:	All
OS:	All
Whiteboard:

Description Mihai Sucan

2012-02-03 12:08:28 EST

In Scratchpad (and soon in the Style Editor) we have a context menu that shows the common Edit menu items (cut/copy/paste/delete/etc).

STR:
1. Download a Firefox nightly, open Scratchpad.
2. Select some text and right-click.

Expected result: you get the Cut, Copy and Delete menu items enabled. Paste should also be enabled if you currently hold some text in the clipboard.

The actual result is that only Copy is enabled.

This happens because the handleBodyMouseDown() function toggles the contentEditable boolean on the clientDiv, only on Firefox. The result is that our platform code is confused during the contextmenu event handling: it can't determine if the element the user right-clicked on is editable or not.

The fix is to add e.which !== 3 to the if condition.

Comment 1 Mihai Sucan

2012-02-03 12:12:38 EST

Proposed fix:

https://github.com/mihaisucan/orion.client/tree/bug-370584

Please review and let me know if this fix is acceptable. Thank you!

This fixes a regression that first showed up in commit 925b8b8408973c9329f324dfef22c697ff505ddc (according to git blame).

Comment 2 Felipe Heidrich

2012-02-06 10:37:04 EST

looks good to me

Comment 3 Mihai Sucan

2012-02-06 12:35:05 EST

Thank you Felipe! I will land the patch!

Comment 4 Felipe Heidrich

2012-02-06 13:31:59 EST

(In reply to comment #3)
> Thank you Felipe! I will land the patch!

I just reviewed the code one more time. I believe, for consistency sake, the _handleBodyMouseUp() should have the same check. Agreed ? If so, please include this change too. 

Other suggestion is 'e.which === 1' instead of 'e.which !== 3'
What do you think ?

Thank you!

Comment 5 Mihai Sucan

2012-02-06 16:00:08 EST

Felipe, that sounds good!

Landed:

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=137d5a8e9bbc0fa204caae74ebd25a7d9d4729bd

Thank you!