Opened 7 years ago

Closed 6 years ago

#5395 closed Bug (fixed)

Opera: right click shows the default context menu

Reported by: brooks Owned by: garry.yao
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.4.2
Component: UI : Context Menu Version: 3.4.2
Keywords: Opera Cc:

Description

just see the attach

Attachments (4)

opera.gif (23.5 KB) - added by brooks 7 years ago.
krst_11.50.28.png (62.1 KB) - added by krst 6 years ago.
Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk
5395.patch (3.2 KB) - added by garry.yao 6 years ago.
5395_2.patch (1.3 KB) - added by garry.yao 6 years ago.

Download all attachments as: .zip

Change History (19)

Changed 7 years ago by brooks

comment:1 Changed 6 years ago by fredck

#5991 has been marked as DUP.

comment:2 Changed 6 years ago by fredck

  • Keywords Opera added
  • Status changed from new to confirmed
  • Summary changed from Opera 10 right click shows the default context menu to Opera: right click shows the default context menu

Changed 6 years ago by krst

Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk

comment:3 Changed 6 years ago by krst

CKEditor menu is covered by opera menu. But sometimes stays, like in #4849 (screenshot where i was able to select option from CKE - menu

comment:4 Changed 6 years ago by krst

  • Component changed from General to UI : Context Menu
  • Priority changed from Normal to High

comment:5 Changed 6 years ago by hallvord@…

I recommend adding some feature detection to this

addTarget:function(l,m){
	if(b.opera){

for example like this:

addTarget:function(l,m){
	if(b.opera&&!('oncontextmenu' in document.createElement('div'))){

comment:6 Changed 6 years ago by hallvord@…

  • Version changed from SVN (CKEditor) - OLD to 3.4.2 (SVN - trunk)

Fred, this is an easy fix and an important bug :)

In plugins/contextmenu/plugin.js, the addTarget function

if ( CKEDITOR.env.opera )

could be for example (slightly different suggestion from above)

if ( CKEDITOR.env.opera && ! ('oncontextmenu') in document.documentElement )

comment:7 Changed 6 years ago by hallvord@…

sorry, should be

if ( CKEDITOR.env.opera && ! ('oncontextmenu' in document.documentElement) )
  • closing parenthesis was too early

comment:8 Changed 6 years ago by fredck

  • Keywords HasPatch added
  • Milestone set to CKEditor 3.4.2

comment:9 Changed 6 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to assigned

DOM2 event feature detecting thanks to hallvord and kangax.

comment:10 Changed 6 years ago by garry.yao

  • Status changed from assigned to review

Changed 6 years ago by garry.yao

comment:11 Changed 6 years ago by Saare

  • Status changed from review to review_failed

I believe that having this big chunk of code just for this small purpose that can be concluded in a much smaller way as @hallvord suggested, is cumbersome and redundant. Using this new method could be considered if we see this kind of feature detecting is needed somewhere else.
Anyway, I'm not able to really test the editor in Opera (duo to an unknown bug at my side), but from cursorily scanning the patch I've noticed that L259-260 could be optimized.

Changed 6 years ago by garry.yao

comment:12 Changed 6 years ago by garry.yao

  • Status changed from review_failed to review

Well Saar, the purpose of introducing such an API is to reduce unreliable browser sniffing in the future, believe me, it will never be really *needed* if it's not presented earlier ;)

comment:13 Changed 6 years ago by fredck

Saar comments are perfect... the first patch is not acceptable really (KISS!!!).

comment:14 Changed 6 years ago by fredck

  • Status changed from review to review_passed

comment:15 Changed 6 years ago by garry.yao

  • Keywords HasPatch removed
  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [5944].

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