Close focused Popup when the main window loses focus

[francoa]

Fixes 91450

Important Note: I've just started checking out Godot's code like a week ago, so it's totally possible that I've failed to notice some unintended consequences of this change proposal. My idea was to close the top-most focused popup whenever the window loses focus.

[AThousandShips]

Your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

[AThousandShips]

This is supposed to be handled in Popup itself, so I think the error here is that the display server doesn't properly deliver the NOTIFICATION_APPLICATION_FOCUS_OUT notification

godot/scene/gui/popup.cpp

Lines 107 to 111 in 06d105e

	case NOTIFICATION_APPLICATION_FOCUS_OUT: {
	if (!is_in_edited_scene_root() && get_flag(FLAG_POPUP)) {
	_close_pressed();
	}
	} break;

It also works correctly on Windows, and I can't find any dedicated code like this on Windows

[francoa]

Your commit seems not to be linked to your GitHub account.

@AThousandShips Thanks for noticing! Email modified and commit signed!

This is supposed to be handled in Popup itself, so I think the error here is that the display server doesn't properly deliver the NOTIFICATION_APPLICATION_FOCUS_OUT notification

Yeah, sounds logical. I tried to follow that route at first, but didn't reach to any conclusion. I'll give it another go

[AThousandShips]

This is indeed the necessary fix, this change would be breaking other things potentially, notice that this:

godot/platform/linuxbsd/x11/display_server_x11.cpp

Lines 4646 to 4651 in 06d105e

	if (!app_focused) {
	if (OS::get_singleton()->get_main_loop()) {
	OS::get_singleton()->get_main_loop()->notification(MainLoop::NOTIFICATION_APPLICATION_FOCUS_IN);
	}
	app_focused = true;
	}

Has no equivalent in FocusOut so that's probably what's missing

Probably something in this code that's broken:

godot/platform/linuxbsd/x11/display_server_x11.cpp

Lines 4280 to 4304 in 06d105e

	if (app_focused) {
	//verify that one of the windows has focus, else send focus out notification
	bool focus_found = false;
	for (const KeyValue<WindowID, WindowData> &E : windows) {
	if (E.value.focused) {
	focus_found = true;
	break;
	}
	}
	if (!focus_found) {
	uint64_t delta = OS::get_singleton()->get_ticks_msec() - time_since_no_focus;
	if (delta > 250) {
	//X11 can go between windows and have no focus for a while, when creating them or something else. Use this as safety to avoid unnecessary focus in/outs.
	if (OS::get_singleton()->get_main_loop()) {
	DEBUG_LOG_X11("All focus lost, triggering NOTIFICATION_APPLICATION_FOCUS_OUT\n");
	OS::get_singleton()->get_main_loop()->notification(MainLoop::NOTIFICATION_APPLICATION_FOCUS_OUT);
	}
	app_focused = false;
	}
	} else {
	time_since_no_focus = OS::get_singleton()->get_ticks_msec();
	}
	}

Possibly related:

• NOTIFICATION_APPLICATION_FOCUS_IN / OUT doesn't trigger with Alt + Tab when a popup is open #63405

[Sauermann]

Related to: #68447

[Sauermann]

This function doesn't need to be public, so it would be better to make it private.
Private functions in this C++ code base are supposed to start with an underscore "_".

Since the return value is not used, the function definition should be void instead of bool.

[AThousandShips]

The current solution is almost certainly not the right one so don't think this detail is something to fix at the moment