wxFlatNotebook update + Patch

[mandrav]

As I expected, the patch doesn't work correctly. That's why I wanted to test it myself.
Steps to reproduce the bug:

1) Right-click on "Build messages".
2) Select "Show/hide->Build log".

The build log hides.

3) Right-click on "Build messages".
4) Select "Show/hide->Build log".

The build log shows again and, seemingly, gets the focus. But still the "Build messages" is displayed.

5) Left-click on "Build messages" and enjoy.

[thomas]

Also it's totaly legal to "delete" a null pointer. All the problems arise from the fact that including myself many forget to set the pointer to null after deletion.

Right. Actually everybody should use something like this all the time:

Code

template<typename T> void Delete(T *p) {delete p; p = 0;};

Hmm... I really wonder why nobody does that. Something must be fundamentally wrong with this, that would be just too simple...  :?:

One could use reference counted smart pointers to avoid above problem, too, of course.
If fact, I believe an object having a Destroy() function should necessarily be reference counted - if it only wraps around a plain normal delete, then it is actually not of much use to have such a function.

[yop]

...Something must be fundamentally wrong with this, that would be just too simple...  :?:

Well there is http://public.research.att.com/~bs/bs_faq2.html#delete-zero but still deleting and setting to 0 should cover 99.99% of the cases.

[MortenMacFly]

Steps to reproduce the bug:

...confirmed. But: How strange is that? I don't even understand what's happening (to be honest) - where is this space coming from suddenly? :shock:
Anyway: I expected that testing is required - that's why I did not file a patch to BerliOS in the first place. Forgive me, but for my "daily work" it really works properly. But I have to admit that the only thing I'm closing/opening are code files - the lower layout I keep as it is normally.
With regards, Morten.

[MortenMacFly]

...alright, I've tried to track down this issue nearly the whole day without success. In my humble opinion this is a very weird bug and from the changes done from 0.90 to 1.30 I really don't understand why and where this bug was introduced. Anyway, I'm not an expert in wxWidgets which makes things for me even worse. If anybody could at least give me a hint... that would be really helpful. I'm closing for today (tonight ;-)), maybe I seee more clear tomorrow... :|
With regards, Morten.

[MortenMacFly]

As I expected, the patch doesn't work correctly. That's why I wanted to test it myself.

mandrav: Correct me, if I'm wrong, but could it be that this bug also appears in the nightlys? Starting from v0.90 (C::B pathhed as from SVN) I tried to apply the changes done up to revision 1.30 to see when the issue appears. But when I compiled C::B again using the SVN version the bug did already appear. I thought it might have been a missing deletion of any file, did a re-build... again. Then I downloaded the latest nightly available and renamed "my" C::B folder. But when I run the nightly and do the steps you described the same thing happens. Are you aware of that?
If so I got you wrong because I thought this issue was introduced somewhere in between v0.90 and v1.30...?!
With regards, Morten.

[PChris]

As I expected, the patch doesn't work correctly. That's why I wanted to test it myself.

mandrav: Correct me, if I'm wrong, but could it be that this bug also appears in the nightlys? Starting from v0.90 (C::B pathhed as from SVN) I tried to apply the changes done up to revision 1.30 to see when the issue appears. But when I compiled C::B again using the SVN version the bug did already appear. I thought it might have been a missing deletion of any file, did a re-build... again. Then I downloaded the latest nightly available and renamed "my" C::B folder. But when I run the nightly and do the steps you described the same thing happens. Are you aware of that?
If so I got you wrong because I thought this issue was introduced somewhere in between v0.90 and v1.30...?!
With regards, Morten.

I can confirm this too...
This bug is also present on the old version.

[Der Meister]

Confirmed even on Linux. Revision 2417, compiled with gcc 3.4.5 and wxGTK 2.6.2 (unicode).

[TheTuxKeeper]

Same with rev2411 on Fedora (wxGTK 2.6.2 unicode + gcc 4.02). If you repeat these steps more than once the space is getting few pixels larger with each repetition of these steps until only few pixels of the headline are visible. Even the vertical scrollbar is gone  :shock:

I forgot to save the last screenshot, but this one is only few times before the space isn't getting bigger anymore.

[Der Meister]

I'm not quite sure what behaviour is intended. But this problem can be fixed with this little patch:

Code

Index: src/sdk/messagemanager.cpp
===================================================================
--- src/sdk/messagemanager.cpp  (revision 2417)
+++ src/sdk/messagemanager.cpp  (working copy)
@@ -332,7 +332,7 @@
     if (show && !ls->visible)
     {
         // show
-        m_pNotebook->InsertPage(id, log, ls->title, false);
+        m_pNotebook->InsertPage(id, log, ls->title, true);
 
         SetLogImage(id, ls->bitmap);
         ls->visible = true;

This way the tab that will be shown again gets selected. I think this is not the originally intended behaviour but it at least works without the problem described here.

[thomas]

Is "selected" synonymous to "given focus"?

I guess if "selected" only means "becomes topmost tab" then it is quite harmless (and even intuitive).

[Der Meister]

Yes, it has the focus, it is displayed and is the topmost tab. Whether this is intuitive or not is another question. I would even prefer that the tab that was selected before stays selected and the "new one" is only placed between the other inactive tabs. I guess that was the originally intended behaviour.
But with a quick look at the roadmap I would say we should apply this little patch (even if the behaviour is not as intuitive than before - that is my opionion, maybe the others think different) as a workaround for this problem rather than spending hours or even days on searching and fixing the real problem.

[mandrav]

HEAD updated to wxFNB 1.30.

[sethjackson]

HEAD updated to wxFNB 1.30.

Yay! 

Aww bugs....

Do this.

1. Right click on a messages tab.
2. Show/Hide a visible tab.
3. Show/Hide the same tab (making it visible).
4. Not good.

Fix.

Code: diff

Index: src/sdk/messagemanager.cpp
===================================================================
--- src/sdk/messagemanager.cpp	(revision 2421)
+++ src/sdk/messagemanager.cpp	(working copy)
@@ -337,8 +337,6 @@
         SetLogImage(id, ls->bitmap);
         ls->visible = true;
 
-        log->Show(false);
-
         if (id == m_DebugLog)
             cfg->Write(_T("/has_debug_log"), (bool)true);
     }
@@ -350,8 +348,6 @@
             m_pNotebook->RemovePage(id);
         ls->visible = false;
 
-        log->Show(false);
-
         if (id == m_DebugLog)
             cfg->Write(_T("/has_debug_log"), (bool)false);
     }

Posted to the tracker.

https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=1038&group_id=5358

[MortenMacFly]

Well... there is something wrong with how this is applied in SVN. Currently the cb_wxfn.patch holds the patch how to update the C::B sources to wxFlatNotebook 1.30-patched. My intention was to provide the patch how to change the original wxFNB sources (v1.30) to the way they are used in SVN.
I've attached a patch to this post (to be applied to the SVN sources) that will:
1.) Update the cb_wxfn.patch file to really have the changes applied within C::B in comparision to wxFNB v1.30
2.) Update the readme file
3.) Apply one minor change to wxFlatNotebook I've missed
With regards, Morten.
Edit: Removed patch to save forum space. It has been applied already.