Opened 8 years ago

Closed 8 years ago

#3373 closed Bug (fixed)

Don't show context menu if there are no items

Reported by: Josh Nisly Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.1
Component: UI : Context Menu Version: SVN (CKEditor) - OLD
Keywords: HasPatch Confirmed Review+ Cc:

Description

If the clipboard plugin is disabled, there are usually no menu items to display in the context menu (there are still items for tables, etc).

In this case, it would be nice to simply display the browser's default context menu, rather than showing an (ugly) empty menu.

Attached is a patch that implements this.

Attachments (7)

contextmenu.patch (2.3 KB) - added by Josh Nisly 8 years ago.
3373.patch (2.3 KB) - added by Garry Yao 8 years ago.
3373_2.patch (2.5 KB) - added by Garry Yao 8 years ago.
3373_3.patch (2.4 KB) - added by Josh Nisly 8 years ago.
Resyncing to current SVN HEAD
3373_4.patch (1.3 KB) - added by Garry Yao 8 years ago.
3373_5.patch (677 bytes) - added by Garry Yao 8 years ago.
3373_6.patch (566 bytes) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (19)

Changed 8 years ago by Josh Nisly

Attachment: contextmenu.patch added

Changed 8 years ago by Garry Yao

Attachment: 3373.patch added

comment:1 Changed 8 years ago by Garry Yao

Keywords: Confirmed Review? added

It's a good move, revising path name.

Changed 8 years ago by Garry Yao

Attachment: 3373_2.patch added

comment:2 Changed 8 years ago by Josh Nisly

Keywords: HasPatch added

comment:3 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.1

Changed 8 years ago by Josh Nisly

Attachment: 3373_3.patch added

Resyncing to current SVN HEAD

Changed 8 years ago by Garry Yao

Attachment: 3373_4.patch added

comment:4 Changed 8 years ago by Garry Yao

Component: GeneralUI : Context Menu
Owner: set to Garry Yao
Status: newassigned
Version: CKEditor 3.0 BetaSVN (CKEditor)

Proposing of showing native when there's no menu items to show. Ticket Test added at :
http://ckeditor.t/tt/3373/1.html.

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I don't think should the browser menu is the right way here. If we do that, sometimes the user will have the browser menu, other times the editor menu, and those are completely different in features and style. It's a matter of consistency.

Other than that, one of the reasons to not give access to the default browser menu is to not give access to options that are not related to the editing area.

So, the proper solution for it is to really not showing anything in such case. I would still recommend leaving the clipboard plugin enabled; in this way the user will always have some feedback from the right click.

comment:6 Changed 8 years ago by Josh Nisly

Keywords: Review? added; Review- removed

Let me try to explain why I think this is useful: in our installation, we want to use the native browser menu most of the time, for things like copy/paste. (No, pasting into a popup dialog doesn't cut it!) However, there are some features, like modifying tables, that need to use CKEditor's menu. So when the user right-clicks on a table, we give them CKEditor's menu to allow them to modify the table; otherwise they get the default browser menu.

I don't think having options that are irrelevant to the editing area is a compelling argument. If the user is used to right-clicking, they are used to having all those right-click options there. Further, when I right-click in Firefox, the only option that might not be relevant is the "Open with" option. All the others are relevant.

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

That's actually one of the points... how to explain users the "Open with" or the "Sent to" options? Or even avoid the "Underline" option that Safari displays, or avoiding its weird way to apply Bold and Italic, all of them in the browser context menu? Saying "please don't touch it" is not a good answer.

Then again, the context menu style... a totally different menu, depending on where you're clicking. This is not usable.

What if someone wants to paste inside a table cell through the context menu? No way... the browser menu with that option will be available only outside the table.

So, what we need is consistency. One must decide whether to show the editor menu or the browser menu, always, or even having a key combination to control it.

Changed 8 years ago by Garry Yao

Attachment: 3373_5.patch added

comment:8 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

Let's consider consistency as the most important thing.

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

There is no clear reason for having setTimout there, as we didn't use it originally. At that point, not even the hasItems variable is needed anymore.

Changed 8 years ago by Garry Yao

Attachment: 3373_6.patch added

comment:10 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:12 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4490].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy