Focus stealing progress dialog

[Axel Spoerl]

@Perdrix said in Focus stealing progress dialog:

Try running the code I pointed you to on Dropbox.

That code doesn't compile without openmp and I don't have the time to re-arrange the code.

Just by code-reading and as a general comment: The progress setters seem to be invoked from different threads, that aren't QThreads, so they have no event loop running. I am not sure, what processEvents() really does in that case.
I don't use openmp. The loop in Dialog::runTask() depends on it and it's not a trivial change to remove that dependency.

As a general question: Why are you struggling to boil the issue down to a simpler reproducer? Have you tried the debugging hints from my previous post?
I'd simply connect a lambda to QApplication::focusChanged(). Call a qFatal() << "Focus thief caught red handed"; inside the lambda, when the old focus object is one of the progress bars. If it crashes too early, use a static int as a counter to crash when focus is stolen. Then you have a perfect stack trace to the point where it happened.

[Perdrix]

I'll post a revised version with openmp removed entirely. Yes it's bad form to do long running processing on the gui thread - but that's what we have so need to call processEvents() from the dlg.

Your suggestions sound good - I wasn't aware of QLoggingCategory stuff and hadn't considered the use of QApplication::focusChanged - that's the advantage you have over me! I'll see what mileage I can get out of those.

D.

[Perdrix]

Hmm odder and odder:
I did this:

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

    Dialog w;

    QObject::connect(&a, &QApplication::focusChanged, &a,  [](QWidget* old, QWidget* now){
        qDebug() << "Old focus widget: " << old;
        qDebug() << "New focus widget: " << now;
            }
        );

    w.show();
    return a.exec();
}

As far as Qt is concerned when I click on another application, focus leaves the "StopButton" (the one named "Cancel" and the focus widget goes to nullptr. BUT the Progress dlg still jumps to the front and steals focus from the other application the instant that the progress is updated. No focusChanged signal occurs when that happens.

Old focus widget:  QPushButton(0x55d1c44c14a0, name="StopButton")
New focus widget:  QWidget(0x0)

What on earth is going on here? The application focus (not the individual Widget focus) IS changed when the progress dlg updates the progress bars.

[Perdrix]

As promised here's the code with openmp stuff removed. It's a tar.gz file setup to build on Linux.

https://www.dropbox.com/scl/fi/ullkvjzhr4qod54wuaako/DSS_ProgressDlg.tar.gz?rlkey=hncz5km5tnxt9vy3axzw3koxy&st=iq8zaagq&dl=0

[Axel Spoerl]

Aha, in that case it's all about the definition of the word "focus".
When QApplication::focusWidget() == nullptr, the application hasn't got focus in the Qt universe.
The application won't receive user input events (e.g. mouse, keyboard).
When updating of the progress bars brings the window to the front, this has nothing to do with focus. It's a change in Z-order, which is sole business of the Window manager. Qt has no say here. My window manager (openSuSE / X11 / KDE Plasma 6) doesn't show that behavior.
Qt doesn't even know the Z order. That's why e.g. QApplication::widgetAt(const QPoint &) relies on the platform layer. On X11, it will ask the X server, for the window under the point.

=> On which system do you observe the Z order change?

[Perdrix]

This happens on Lubuntu 22.04 and on macOS (at least on Ventura and later).

I think it also happens on some levels of W11.

It's more than just Z-Order - if I were typing in the other window the keystrokes are lost after the dlg is updated.

What I don't understand is why doesn't this happen with QProgressDialog, but does with our dialogue.

[Axel Spoerl]

It happens in the ProcessDialog class, because it's implemented that way.
The methods below are called on each update of the progress bar.
While the method name suggests, that they are called only once at the start, they are actually hit on every update.
Calling raise() raises them.

void ProgressDlg::applyStart1Text(const QString& strText)
{
	ui->ProcessText1->setText(strText);
	raise(); // <----- HERE
}

void ProgressDlg::applyStart2Text(const QString& strText)
{
	ui->ProcessText2->setText(strText);
	setProgress2Range(0, m_total2);
	if (m_total2 == 0)
	{
		setItemVisibility(true, false);
	}
	else
	{
		setItemVisibility(true, true);
		applyProgress2(0);
	}
	raise(); // <----- AND HERE
}
``

[Axel Spoerl]

Please forgive me two general coding remarks:

• setItemVisibility(true, false) is a bool trap. Impossible to guess what it means, you always have to read the implementation. I'd always use an enum instead, to make it more guessable. Zero cost in the executable, the compiler will optimize it.
• Calling a non static method of an object living in another thread isn't a good idea. void ProgressDlg::Progress1() for instance has the sole purpose to invoke slotProgress1(). Since ProgressDlg inherits from QObject anyway, why not make Progress1 a slot? It could be either directly invoked from another thread, or just connected to a signal that you could emit and not worry about threads altogether.

[Perdrix]

Thanks ever so much!!! You are 100% correct it was the raise() call that was causing all the problems - me failing to see the wood for the trees I fear.

I have no skin in this code I didn't write it :) - just trying to fix it. Even if I had written it though, all suggestions are welcome

I'll get the developer who wrote it to see what can be done to change the code to use emit to drive the dialog updates.

[Axel Spoerl]

Well, David, we've all been there, have we not? The focus theft topic caused me a slight panic wave: I re-factored the focus chain handling in widgets a while ago. What looked like piece of beauty, introduced some awful regressions. And since you've caught me red handed on a dock widget regression earlier, I smelled trouble coming my way ;-)