Opened 9 years ago

Closed 8 years ago

#1982 closed Bug (fixed)

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

Reported by: w.olchawa Owned by: martinkou
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 alfonsoml 8 years ago.
Proposed patch
1982_1.patch (3.7 KB) - added by alfonsoml 8 years ago.
Patch including RTL handling.
1982_2.patch (1.6 KB) - added by martinkou 8 years ago.
Modified Alfonso's patch to support more than 2 submenu levels.
1982_3.patch (1.7 KB) - added by martinkou 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by fredck

  • Milestone set to FCKeditor 2.7
  • Version changed from FCKeditor 2.6 Beta to FCKeditor 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 8 years ago by alfonsoml

Proposed patch

comment:2 Changed 8 years ago by alfonsoml

  • Owner set to alfonsoml
  • Status changed from new to assigned

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 8 years ago by alfonsoml

Patch including RTL handling.

comment:3 Changed 8 years ago by alfonsoml

  • Component changed from General to UI : 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 8 years ago by martinkou

  • Keywords Review+ added; Review? removed

comment:5 Changed 8 years ago by alfonsoml

  • Milestone set to FCKeditor 2.6.4
  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [2386]

comment:6 Changed 8 years ago by AlfK

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:7 Changed 8 years ago by alfonsoml

  • Keywords Review+ removed
  • Owner alfonsoml deleted
  • Status changed from reopened to new

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 8 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:9 Changed 8 years ago by martinkou

  • 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 8 years ago by martinkou

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

comment:10 Changed 8 years ago by fredck

  • 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 8 years ago by martinkou

comment:11 Changed 8 years ago by martinkou

  • Keywords Review? added; Review- removed

Added the cleanup code.

comment:12 Changed 8 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:13 Changed 8 years ago by martinkou

Fixed with [2597].

Click here for more info about our SVN system.

comment:14 Changed 8 years ago by martinkou

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy