Solving the problem of launching the settings dialog without xine support #14

The settings dialog crashes if built without xine support. These changes do not affect the recording process.
These changes solve the problem of opening the settings dialog box if the build is carried out without xine support.

The main reason for adding this is that the recorderClicked slot only applies to xine. File src/pref.cpp:

KDE_NO_EXPORT void PrefRecordPage::recorderClicked (int id) {
    bool b = recorder->find(id)->text().find (TQString::fromLatin1("Xine")) > -1;
    replay->setEnabled (!b);
    if (b)
        replay->setButton (Settings::ReplayNo);

}

In addition, in this file this slot is already bound to the HAVE_XINE macro:

#ifdef HAVE_XINE
    connect (recorder, TQT_SIGNAL (clicked(int)), this, TQT_SLOT(recorderClicked(int)));
#endif

It seems that in the src/kmplayerconfig.cpp file someone forgot to do the same for the recorderClicked slot.

I am interested to understand where the crash happens. Is it because recorder->find(id) returns a null pointer? In that case it may be more appropriate to check for that and return from the call instead of introducing a #ifdef block (we could in fact remove the original one too).
Using #ifdef blocks to protect from crashes is not really good practice, because it only hides a problem away rather than fixing it properly. If someone uses a different settings for some #define, the problem could show up again.

I am interested to understand where the crash happens. Is it because recorder->find(id) returns a null pointer?

Yes, sure.link
The problem occurs when the ~/.trinity/share/config/kmplayerrc file has not yet been created.
An option without macros might look like this:

--- a/src/pref.cpp	2023-11-28 08:00:37.426857575 +0300
+++ b/src/pref.cpp	2023-11-28 08:04:58.909846639 +0300
@@ -475,7 +475,10 @@
 }
 
 KDE_NO_EXPORT void PrefRecordPage::recorderClicked (int id) {
-    bool b = recorder->find(id)->text().find (TQString::fromLatin1("Xine")) > -1;
+    bool b = false;
+    if (recorder->find(id)) {
+        b = recorder->find(id)->text().find (TQString::fromLatin1("Xine")) > -1;
+    }
     replay->setEnabled (!b);
     if (b)
         replay->setButton (Settings::ReplayNo);

That's a better solution IMO. You can improve even further by avoiding a double call to find():

bool b = false;
TQButton *recBtn = recorder->find(id);
if (recBtn)
{
  b = recorder->find(id)->text().find (TQString::fromLatin1("Xine")) > -1;
}

You can now try to remove the #ifdef HAVE_XINE in src/pref.cpp:438 and see if all still works fine.

Btw, I could not access the link above, but I think we are both clear on the solution now :-)

You can now try to remove the #ifdef HAVE_XINE in src/pref.cpp:438 and see if all still works fine.

Ok, I'll add it now and check it. But I already think that everything will be okay.

tw, I could not access the link above, but I think we are both clear on the solution now :-)

Now I also cannot access this information, apparently there are some problems with the server. Here I re-uploaded it. Link

Ok, I'll add it now and check it. But I already think that everything will be okay.

Excellent. If you update the patch, then we can approve and merge it in the new form.

Added changes, checked it works!

Patch looks good and well prepared. One last touch: can you remove the #ifdef HAVE_XINE in src/pref.cpp:438 if it doesn't cause any side effect?

Okay, I'll test this with an additional call to connect.

Patch looks good and well prepared. One last touch: can you remove the #ifdef HAVE_XINE in src/pref.cpp:438 if it doesn't cause any side effect?

I added the changes, everything works fine. No side effects occurred after adding these changes.

Now all looks good. We removed a bad hackish solution (the #ifdef) and replace it with a proper fix 🙂

Merged and backport. Thanks @ormorph for the patch, the testing and the patience to update through the process.

Solving the problem of launching the settings dialog without xine support #14

Reviewers