Fix bug on x11 where window close events were not propagated with certain window styles

[danieljpetersen]

Description

This is intended to fix #2943.

It's a single line change which adds a flag for x11 telling it to propagate close events for styles None, Resize, Titlebar.

In truth I do not understand x11 very well, but the behavior / fix seems to make sense to me at least. It fixes the bug on my machine, and doesn't visually impact the window decorations.

I've only confirmed the initial bug on Cinnamon. Flareon from the Discord confirmed it was present on KDE.

I've only tested this PR Cinnamon.

How to test this PR?

Make sure hotkey is bound which closes in-focus window, compile / run this code, press hotkey
A) without this PR, the window will not get sf::Event::Close, the window will never close.
B) with this PR, the window will get sf::Event::Close, the window will successfully close.

#include <SFML/Window.hpp>
 
int main()
{
    sf::Window window; // also happens with RenderWindow
 
    // The following are all not working: Titlebar, Resize, None
    window.create(sf::VideoMode({800, 600}), "My window", sf::Style::None);
 
    while (window.isOpen()) {
        sf::Event event{};
        while (window.pollEvent(event)) {
            // sf::Event::Closed is never given to us from window.pollEvent(event)
            if (event.type == sf::Event::Closed)
                window.close();
        }
 
        window.display();
    }
 
    return 0;
}

[danieljpetersen]

Ah, hmm, this actually alters the the decorations for resize and titlebar. I only tested against none before submitting the PR, should have tested all three cases.

With that in mind then, I could alter the PR so the bug is at least fixed for style::None, but I'm not sure how you'd fix it for the other two.

[coveralls]

Pull Request Test Coverage Report for Build 8681308266

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 52.978%

---

Totals
Change from base Build 8675259711:	0.0%
Covered Lines:	10331
Relevant Lines:	18496

---

💛 - Coveralls

[eXpl0it3r]

Ah, hmm, this actually alters the the decorations for resize and titlebar. I only tested against none before submitting the PR, should have tested all three cases.

How is it changed?

[danieljpetersen]

Ah, hmm, this actually alters the the decorations for resize and titlebar. I only tested against none before submitting the PR, should have tested all three cases.

How is it changed?

It adds the close button to the window decorations for resize and titlebar. sf::Style::None however remains without decorations.

So the PR as it currently stands has regressions for resize and titlebar. We could modify the PR to just target sf::Style::None. This would effectively fix the bug for the sf::Style::None window state with no regressions. sf::Style::Resize and sf::Style::Titlebar would obviously remain broken if we did this (unable to close via hotkey). I do not know if there is a way to actually fix the bug for these two states.

To be as clear as possible,

• sf::Style::None
  • Without current PR:
    • No window decorations
    • does not close via quit hotkey
  • With current PR:
    • No window decorations
    • Closes via window hotkey
• sf::Style::Resize
  • Without current PR:
    • Has window decorations with minimize, maximize
    • Does not close via quit hotkey
  • With current PR:
    • Has window decorations with minimize, maximize, close
    • Closes via quit hotkey
• sf::Style::Titlebar
  • Without current PR:
    • Has window decorations with minimize
    • Does not close via quit hotkey
  • With current PR:
    • Has window decorations with minimize, close
    • Closes via quit hotkey

I'm sure this is unnecessary, but to be abundantly clear here are some images

None with PR as it currently stands:

None without PR as it currently stands:

Resize with PR as it currently stands (regression, erroneously adds close):

None without PR as it currently stands:

Titlebar with PR as it currently stands (regression, erroneously adds close):

Titlebar without PR as it currently stands: