Ticket #1838 (closed Bug: fixed)

Opened 7 years ago

Last modified 4 years ago

Context-menu doesn't aways dissapear after selecting an option

Reported by: dustball Owned by: martinkou
Priority: Normal Milestone: FCKeditor 2.6
Component: UI : Context Menu Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Firefox Review+ Cc:

Description (last modified by alfonsoml) (diff)

Tested with Firefox:

  1. Load the nightly demo at http://www.fckeditor.net/nightly/fckeditor/_samples/default.html
  2. Insert a table
  3. Right click in a cell, choose Table Properties
  4. Repeat #3 up to ten times (usually happens on fourth try?)

Observation: the right click menu doesn't go away after selecting Table Properties

Attachments

1838.patch (2.0 KB) - added by martinkou 7 years ago.
1838_2.patch (2.1 KB) - added by martinkou 7 years ago.

Change History

comment:1 Changed 7 years ago by alfonsoml

  • Keywords Confirmed Firefox added
  • Version set to SVN
  • Description modified (diff)

Confirmed with Firefox 2.0.0.12, seems to work fine with the rest of browsers.

Instead of several tries, I've failed as soon as I clicked for example in the "copy" option (that was disabled), and then I couldn't make the context menu go away

comment:2 Changed 7 years ago by alfonsoml

  • Milestone set to FCKeditor 2.6

It's annoying, targeting for 2.6

comment:3 Changed 7 years ago by alfonsoml

#1976 has been marked as dup

comment:4 Changed 7 years ago by martinkou

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

comment:5 Changed 7 years ago by dustball

We've gotten six bug reports about this at PBwiki, where we have the Beta deployed in our Beta :)

comment:6 Changed 7 years ago by martinkou

I've seen it happening a few times on Windows/Firefox2 (not once on Mac/Firefox2) today, but whenever I wanted to reproduce that, it just seems to not happen :/

There's definitely a problem here, but reproducing it consistently is kinda hard. This might take a few days to fix.

comment:7 Changed 7 years ago by martinkou

I think I've got better luck reproducing the issue when my mouse hovered above the "Cell", "Row" and "Column" before clicking on "Table Properties". But still I'm only getting the bug every 20 or 30 trials.

But I guess I've got the gist of the issue now, it seems there's a synchronization problem with the FCKPanel::_LockCounter variable when it's being incremented and decremented quickly from different events - the pattern of counter values I've got from the debug messages of the counter looks very much like the "too much milk problem" counter taught in threaded programming classes.

comment:8 Changed 7 years ago by martinkou

I still haven't solved the bug today (just doing tests takes a lot of time already). But now it seems it is not a synchronization issue on the FCKPanel::_LockCounter property now.

Some more tests today showed that whenever the bug occurred, FCKPanel::Lock() gets called 4 times while FCKPanel::Unlock() gets called only 3 times, leave FCKPanel::_LockCounter = 1 when the Table Properties item is clicked and thus the menu wouldn't close. Normally, both Lock() and Unlock() should get called only 3 times (1 time for each open/closing of the three submenus above "Table Properties"). I still haven't pinned down how come Lock() would get called the fourth time.

comment:9 Changed 7 years ago by martinkou

I think I've finally found out why the menus aren't closing and derived a fix for that. Although with this kind of bug it can never be 100% sure since I don't know whether it will appear again at the 1000th trial to reproduce it despite having the fix. Anyway, here's my theory on why it is happening, based on what I've seen in today's code traces:

If you look into FCKPanel::Show() you'll see there's a little closure called "resizeFunc()", which sets the flag FCKPanel::_IsOpened to true. It is not executed directly by FCKPanel::Show(), however. It is only executed with a setTimeout() with a 1ms delay. So there's a 1ms delay (probably much longer, as you know the JavaScript timer isn't very accurate) during which the _IsOpened state is desynced from the rest of the FCKDialog object's states.

Now what can be checking that _IsOpened flag during the 1ms window? My code trace today showed that FCKMenuBlockPanel::Show(), which is called when opening the Row, Cell, Column submenus, would check it by calling FCKPanel::CheckIsOpened(). What happened, at least during the code traces I've seen, was that FCKMenuBlockPanel::Show() may get called multiple times even for the same submenu while your mouse hovered above the submenu's parent item quickly. The first time it is called, the submenu's FCKPanel::Show() is called, a timer is set to run resizeFunc() and the parent menu's _LockCounter is incremented. The second time it is called, it finds that the submenu is still not opened (since the timer has not run resizeFunc() yet), and thus called FCKPanel::Show() for the submenu again. Which, subsequently, causes the _LockCounter of the parent menu to be incorrectly incremented by 1 again.

So the fix to this, would be to move the _IsOpened = true assignment out of the resizeFunc() closure and make it executed with the rest of the code in FCKPanel::Show(). This creates a potential problem, however, that it makes it possible for a submenu to appear after it's FCKPanel::Hide() had been executed during the 1ms window. So we need to take care of that case in FCKPanel::Hide() by clearing the timer used to execute resizeFunc().

Changed 7 years ago by martinkou

comment:10 Changed 7 years ago by martinkou

  • Keywords Review? added

Changed 7 years ago by martinkou

comment:11 Changed 7 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:12 Changed 7 years ago by martinkou

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed with [1730].

Click here for more info about our SVN system.

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