Opened 14 years ago

Closed 13 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 14 years ago.
krst_11.50.28.png (62.1 KB) - added by Krzysztof Studnik 14 years ago.
Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk
5395.patch (3.2 KB) - added by Garry Yao 13 years ago.
5395_2.patch (1.3 KB) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (19)

Changed 14 years ago by brooks

Attachment: opera.gif added

comment:1 Changed 14 years ago by Frederico Caldeira Knabben

#5991 has been marked as DUP.

comment:2 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Opera added
Status: newconfirmed
Summary: Opera 10 right click shows the default context menuOpera: right click shows the default context menu

Changed 14 years ago by Krzysztof Studnik

Attachment: krst_11.50.28.png added

Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk

comment:3 Changed 14 years ago by Krzysztof Studnik

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 14 years ago by Krzysztof Studnik

Component: GeneralUI : Context Menu
Priority: NormalHigh

comment:5 Changed 14 years ago by Hallvord R. M. Steen (Opera Software)

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 13 years ago by Hallvord R. M. Steen (Opera Software)

Version: SVN (CKEditor) - OLD3.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 13 years ago by Hallvord R. M. Steen (Opera Software)

sorry, should be

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

comment:8 Changed 13 years ago by Frederico Caldeira Knabben

Keywords: HasPatch added
Milestone: CKEditor 3.4.2

comment:9 Changed 13 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedassigned

DOM2 event feature detecting thanks to hallvord and kangax.

comment:10 Changed 13 years ago by Garry Yao

Status: assignedreview

Changed 13 years ago by Garry Yao

Attachment: 5395.patch added

comment:11 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_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 13 years ago by Garry Yao

Attachment: 5395_2.patch added

comment:12 Changed 13 years ago by Garry Yao

Status: review_failedreview

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 13 years ago by Frederico Caldeira Knabben

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

comment:14 Changed 13 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:15 Changed 13 years ago by Garry Yao

Keywords: HasPatch removed
Resolution: fixed
Status: review_passedclosed

Fixed with [5944].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy