#1838 closed Bug (fixed)
Context-menu doesn't aways dissapear after selecting an option
Reported by: | Brian Klug | Owned by: | Martin Kou |
---|---|---|---|
Priority: | Normal | Milestone: | FCKeditor 2.6 |
Component: | UI : Context Menu | Version: | SVN (FCKeditor) - Retired |
Keywords: | Confirmed Firefox Review+ | Cc: |
Description (last modified by ) ¶
Tested with Firefox:
- Load the nightly demo at http://www.fckeditor.net/nightly/fckeditor/_samples/default.html
- Insert a table
- Right click in a cell, choose Table Properties
- Repeat #3 up to ten times (usually happens on fourth try?)
Observation: the right click menu doesn't go away after selecting Table Properties
Change History (14)
comment:1 Changed 17 years ago by
Description: | modified (diff) |
---|---|
Keywords: | Confirmed Firefox added |
Version: | → SVN |
comment:4 Changed 17 years ago by
Owner: | set to Martin Kou |
---|---|
Status: | new → assigned |
comment:5 Changed 17 years ago by
We've gotten six bug reports about this at PBwiki, where we have the Beta deployed in our Beta :)
comment:6 Changed 17 years ago by
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 17 years ago by
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 17 years ago by
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 17 years ago by
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 17 years ago by
Attachment: | 1838.patch added |
---|
comment:10 Changed 17 years ago by
Keywords: | Review? added |
---|
Changed 17 years ago by
Attachment: | 1838_2.patch added |
---|
comment:11 Changed 17 years ago by
Keywords: | Review+ added; Review? removed |
---|
comment:12 Changed 17 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed with [1730].
Click here for more info about our SVN system.
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