Wireshark-dev: [Wireshark-dev] How to disable controls in dialog when capture file is closing

From: Jirka Novak <j.novak@xxxxxxxxxxxx>
Date: Wed, 6 Jan 2021 23:35:01 +0100
Hi,

  I would like to disable a button in dialog (QT UI) when capture file
is closing. I studied existing code, reused pattern other dialogs use
and I'm afraid it doesn't work as intended. I mean disabling of
buttons/controls doesn't work in any child of WiresharkDialog.
  Can I ask for verification and help?

  Basic idea is that update of UI is done with updateWidgets(). When
capture file is closed, file_closed_ variable is set to true and UI is
redrawn.
 Therefore pattern is:

updateWidgets() {
...
  button->setEnabled(!file_closed_);
...
}

  I found that it doesn't work. E.g. try existing code:
1) open any capture with TCP packets (just to be able to use dialog I
propose)
2) open Statistics->Conversations, select TCP Tab and click on one row.
Follow Stream button is enabled.
3) Now close the capture file. Header of dialog changes to [no capture
file], but Follow Stream button is still enabled.
4) You have to click on other row in list and Follow Stream button is
disabled now.

  You can repeat this procedure with any dialog which uses buttons in
many stable versions in past (for sure to 2019).
  My first worry was that I made mistake in code, but after checking the
code I'm afraid that framework is wrong...

  How it is indended to work:
  When capture file is closed, WiresharkDialog::captureEvent() receives
event CaptureEvent::Closing. Based on this event captureFileClosing() is
called. It calls current class captureFileClosing(), all its predecessor
and then WiresharkDialog::captureFileClosing() {
{
    if (file_closed_)
        return;

    removeTapListeners();
    updateWidgets();
}

  Therefore I learn that if I overwrite captureFileClosing() it would work:

captureFileClosing() {
  button->setEnabled(false);
}

  but it doesn't. Technically it does, but right after it
updateWidgets() is called from WiresharkDialog::captureFileClosing() and
it does:

  button->setEnabled(!file_closed_);

  and because file_closed_ is not set yet, button is enabled again.
  This is reason why it doesn't work in Statistics dialog (except
Statistics dialog reenables the button in different method then
updateWidgets(), but reason/dependency is the same).

  When I studied the code deeper, I found that there is:

void WiresharkDialog::captureEvent(CaptureEvent e) {
...
    case CaptureEvent::File:
        switch (e.eventType())
        {
        case CaptureEvent::Closing:
            captureFileClosing();
            break;
        case CaptureEvent::Closed:
            captureFileClosing();
            file_closed_ = true;
            break;
        default:
            break;
        }
...
}

  There you can see that captureFileClosing() is called before
file_closed_ is set and therefore any updateWidgets() which checks
file_closed_ can't disable button during captureFileClosing(). And in
real, many of dialogs do not disable buttons correctly nowadays even
it's code do its best.
  I was surprised how it happend and I found that in past file_closed_
was set before captureFileClosing() and there were two methods called:
captureFileClosing() and captureFileClosed(). They were removed during
optimalizations of the code and I'm afraid it broken the framework.

  Did I missed something? Should I try fix the framework? Is there
anyone who remembers why framework was changed to current state?

				Best regards,

							Jirka Novak