Opened 9 years ago

Closed 9 years ago

#1982 closed Bug (fixed)

[IE7] Table context menu submenus doesn't appear properly.

Reported by: Wojciech Olchawa Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.6.4
Component: UI : Context Menu Version: FCKeditor 2.4.3
Keywords: Confirmed IE Review+ Cc:

Description

Steps to reproduce:

1. Go to our demo page http://www.fckeditor.net/demo

2. Insert a table.

3. Right-click inside a cell to activate the context menu

4. Go to a sub-menu e.g. Cell, Row or Column

Notice that the submenu appears not besides the context menu but practically on it.

Expected behavior would be displaying a submenu besides the original context menu.

Tested using 2.6 beta and SVN version on IE7. It works fine in IE6 and FF2.

Attachments (4)

1982.patch (3.2 KB) - added by Alfonso Martínez de Lizarrondo 9 years ago.
Proposed patch
1982_1.patch (3.7 KB) - added by Alfonso Martínez de Lizarrondo 9 years ago.
Patch including RTL handling.
1982_2.patch (1.6 KB) - added by Martin Kou 9 years ago.
Modified Alfonso's patch to support more than 2 submenu levels.
1982_3.patch (1.7 KB) - added by Martin Kou 9 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by Frederico Caldeira Knabben

Milestone: FCKeditor 2.7
Version: FCKeditor 2.6 BetaFCKeditor 2.4.3

This is not a regression. I've confirmed it with version 2.4.3, so it's not a regression.

Even if annoying, it is not a big issue, fortunately.

Changed 9 years ago by Alfonso Martínez de Lizarrondo

Attachment: 1982.patch added

Proposed patch

comment:2 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

I've managed to create a patch that fixes the problem with almost no drawbacks.

The problem is that the submenus are created having the main menu as its parent window, so their dimensions are restricted by that main menu. I've coded a little logic that takes care to increase the size of that panel and so it allows the submenu to have whatever dimensions are needed.

I'm checking now (I wanted to do it earlier but I forgot) that the behaviour with the Hebrew language isn't too nice. I'll have to work on it before requesting review (anyway, if someone has any idea I'll be glad)

Changed 9 years ago by Alfonso Martínez de Lizarrondo

Attachment: 1982_1.patch added

Patch including RTL handling.

comment:3 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Component: GeneralUI : Context Menu
Keywords: Review? added

Ok, I needed to rest a little to find the solution but now it works fine in all my tests.

The only drawback of the patch is that there's a white area behind the submenu, but I think that it's a very minor thing comparing to how hard is currently to access the submenus in IE7 sometimes (for example if you are at the right side of the screen).

comment:4 Changed 9 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:5 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Milestone: FCKeditor 2.6.4
Resolution: fixed
Status: assignedclosed

Fixed with [2386]

comment:6 Changed 9 years ago by Alf Magne Kalleland

Resolution: fixed
Status: closedreopened

comment:7 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ removed
Owner: Alfonso Martínez de Lizarrondo deleted
Status: reopenednew

I have no plans to work on three levels menus as I haven't seen any patch to add such menus or any plugin that uses them.

Maybe someone else wants to spend their time fixing that problem. Clearing out the review so it doesn't show in the default queries also.

comment:8 Changed 9 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:9 Changed 9 years ago by Martin Kou

Keywords: Review? added

It took me a while to figure out what Alfonso's patch was doing, but turns out the modifications needed aren't big.

Changed 9 years ago by Martin Kou

Attachment: 1982_2.patch added

Modified Alfonso's patch to support more than 2 submenu levels.

comment:10 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The patch looks good. There is just one small detail. Please cleanup the this.RelativeElement property, because it could easily result on memory leak.

Changed 9 years ago by Martin Kou

Attachment: 1982_3.patch added

comment:11 Changed 9 years ago by Martin Kou

Keywords: Review? added; Review- removed

Added the cleanup code.

comment:12 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:13 Changed 9 years ago by Martin Kou

Fixed with [2597].

Click here for more info about our SVN system.

comment:14 Changed 9 years ago by Martin Kou

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