Wireshark-dev: Re: [Wireshark-dev] Should Qt SimpleDialog messages be posted to event queue?
On Thu, May 9, 2019 at 2:12 AM Peter Wu <peter@xxxxxxxxxxxxx> wrote:
>
> On Wed, May 01, 2019 at 12:23:16PM +0200, Tomasz Moń wrote:
> > One approach to solve the problem of "unwanted interruptions" would be
> > to change simple_dialog() function to post user-defined event to the
> > event queue. This would avoid the problem by limiting the number of
> > places where the events from the event queue can be handled. In
> > opinion such change impacts the overall user interface architecture.
Posting the messages to queue is a bit insane. A much better approach is
to simply dynamically create the QMessageBox, set the Qt::WA_DeleteOnClose
attribute, set the dialog to be modal, and then call the show() method
instead of exec().
I am glad I didn't just hop into my original event idea as that'd be in
my opinion, complete waste of time.
I have uploaded my solution to bug 15743 to gerrit [1].
> > What do you think about changing simple_dialog() to be asynchronous?
> > Is there some better approach?
>
> I think it a reasonable idea, hopefully there are no significant cases
> where the caller expected it to block though. Obviously memory needs to
> be duplicated, but aside from that we also have a case like
> ui/qt/preference_editor_frame.cpp.
Apparently, the simple_dialog() is not meant to be used in Qt code.
There is following comment at top of ui/qt/simple_dialog.cpp:
/* Simple dialog function - Displays a dialog box with the supplied message
* text.
*
* This is meant to be used as a backend for the functions defined in
* ui/simple_dialog.h. Qt code should use QMessageBox directly.
*
* Args:
* type : One of ESD_TYPE_*.
* btn_mask : The value passed in determines which buttons are displayed.
* msg_format : Sprintf-style format of the text displayed in the dialog.
* ... : Argument list for msg_format
*/
Moreover, there are a lot of calls to simple_dialog() in Wireshark codebase
which makes such drastic change really risky.
Best Regards,
Tomasz Moń
[1] https://code.wireshark.org/review/33205
