Context menu is incorrectly positioned when opened with Shift-F10

[earshinov]

Are you reporting a feature request or a bug?

Bug.

I have not found any relevant issues mentioning context menu and F10.

Reproduction steps

1. Find any CKEDITOR with some text. The editor from CKEDITOR 4 homepage will do.
2. Position the text cursor anywhere in the text inside the editor.
3. Press Shift-F10.

Expected result

The context menu should appear next to the text cursor.

Actual result

The context menu appears somewhere else. In case of the editor on the homepage the menu is opened close to the top left corner of the contents of the editor.

Other details

• Browser: Google Chrome 63.0.3239.132
• OS: Windows 7 x64
• CKEditor version: the issue reproduces in 4.5.9, 4.8.0
• Installed CKEditor plugins: tested in the "standard" distribution of CKEDITOR

[earshinov]

CKEDITOR handles Shift-F10 and Ctrl-Shift-F10 itself instead of relying on the browser.

contextmenu/plugin.js:

editor.setKeystroke( CKEDITOR.SHIFT + 121 /*F10*/, 'contextMenu' );
editor.setKeystroke( CKEDITOR.CTRL + CKEDITOR.SHIFT + 121 /*F10*/, 'contextMenu' );

Actually, many browsers support these shortcuts out of the box, emitting contextmenu event. In such browsers, everything would work fine if CKEDITOR didn't override the default browser behaviour. I have checked latest Chrome, Firefox, and Microsoft Edge - only the latest does not support Shift-F10.

May it be possible to employ browser detection here, perhaps enabling custom handling only in env.ie? Of course one needs to be sure that everything works in older Chrome, Firefox, Safari etc.

[mlewand]

Thnx @earshinov, I'm able to reproduce the issue. Preferred option is to have the same logic for all browsers, without sniffing - so if the positioning can be fixed let's do this and set it for all browsers.

If that would be for some reason impossible, then we can think about falling back to browser sniffing.

Workaround

Based on your preference and your feedback, what I can suggest is to disable our custom handling for non IE browsers. Add below code to your config.js:

	if ( !CKEDITOR.env.ie ) {
		config.keystrokes = [
			[ CKEDITOR.SHIFT + 121 /*F10*/, null ]
		]
	}

[earshinov]

@mlewand, Yes, this is exactly the solution we use. Thankfully, we have CKEDITOR running on a page inside embedded Chromium (CEF), so we can rely on the browser engine to handle these shortcuts for us.

[earshinov]

There is another possible solution I would like to suggest.

Presently, floatpanel plugin, which does the actual positioning for the context menu, just assumes offsetX and offsetY to be 0 when they are unavailable:

floatpanel/plugin.js:

showBlock: function( /* ... */ ) {
	// ...
	left = position.x + ( offsetX || 0 ) - positionedAncestorPosition.x,
	top = position.y + ( offsetY || 0 ) - positionedAncestorPosition.y;
	// ...
}

This is obviously incorrect. Instead, when mouse coordinates are unavailable, as in keyboard event handlers, CKEDITOR could rely on the position of the text cursor. I guess, Range.getClientRects() can be used for this purpose, which happens to be failrly cross-browser.

[mlewand]

@earshinov thanks for a nice investigation. The fix should be applied in contextmenu plugin and picking position from a range seems like a good idea 👍

The fallback to 0 in the generic plugin is ok, as it's not only used by context menu, but also other ui plugins.

I can see that getClientRects() is not supported on IE8, which is not a show stopper for us. It might be worth checking if there are other APIs in IE8 to get it working there too, but if not, it could just keep fallback x,y=0 values.

[Comandeer]

Merged into next, will be released in 4.10.2.

[mlewand]

Note that you can already have a peek at the working solution in our nightly preview 😉