Opened 13 years ago
Closed 12 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)
Change History (19)
Changed 13 years ago by
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
| Keywords: | Opera added |
|---|---|
| Status: | new → confirmed |
| Summary: | Opera 10 right click shows the default context menu → Opera: right click shows the default context menu |
Changed 13 years ago by
| Attachment: | krst_11.50.28.png added |
|---|
Confirmed for Opera 10.61.3484, CKE 3.4.1 Trunk
comment:3 Changed 13 years ago by
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 13 years ago by
| Component: | General → UI : Context Menu |
|---|---|
| Priority: | Normal → High |
comment:5 Changed 13 years ago by
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
| Version: | SVN (CKEditor) - OLD → 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 13 years ago by
sorry, should be
if ( CKEDITOR.env.opera && ! ('oncontextmenu' in document.documentElement) )
- closing parenthesis was too early
comment:8 Changed 12 years ago by
| Keywords: | HasPatch added |
|---|---|
| Milestone: | → CKEditor 3.4.2 |
comment:9 Changed 12 years ago by
| Owner: | set to Garry Yao |
|---|---|
| Status: | confirmed → assigned |
DOM2 event feature detecting thanks to hallvord and kangax.
comment:10 Changed 12 years ago by
| Status: | assigned → review |
|---|
Changed 12 years ago by
| Attachment: | 5395.patch added |
|---|
comment:11 Changed 12 years ago by
| Status: | review → 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 12 years ago by
| Attachment: | 5395_2.patch added |
|---|
comment:12 Changed 12 years ago by
| Status: | review_failed → 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 12 years ago by
Saar comments are perfect... the first patch is not acceptable really (KISS!!!).
comment:14 Changed 12 years ago by
| Status: | review → review_passed |
|---|
comment:15 Changed 12 years ago by
| Keywords: | HasPatch removed |
|---|---|
| Resolution: | → fixed |
| Status: | review_passed → closed |
Fixed with [5944].
#5991 has been marked as DUP.