Details
- Reviewers
hein graesslin - Commits
- R278:6178ab9b4006: Add "SkipSwitcher" to API
Diff Detail
- Repository
- R278 KWindowSystem
- Branch
- skip-switcher (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
After discussion in BUG 375921, it was decided to add SkipSwitcher to the KWin API. It's a junior job and my first submission to the KWin/KWayland/KWindowSystem family. @hein has agreed to help me out, and I do in fact have a few issues.
- Autotests 7, 8, and 10 are failing, and I'm unable to determine why. It appears to be somehow related to the addition of NET::SkipSwitcher to the list of NET::States, but I'm unclear why or how.
- I know KWin/X11 is feature-frozen, so this may not be relevant any longer.
As Martin wrote in the bug report, this should be straightforward, and it seems that way. But clearly the results of the autotests are a problem, which I need help resolving.
| src/netwm_def.h | ||
|---|---|---|
| 492 | add @since 5.45? | |
| src/netwm_def.h | ||
|---|---|---|
| 492 | Will it be 5.45 for KWayland as well? There were @since tags in what I duplicated, but I wasn't certain which version. | |
5.44 was just released, so I'd assume it's 5.45 (or we can fix change it later if it takes us longer to merge).
I took a minute to think it through and came to the conclusion of 5.45 as well.
I'll update.
I'm not sure whether we can add elements to the states.
| autotests/netwininfotestwm.cpp | ||
|---|---|---|
| 432 ↗ | (On Diff #31307) | This needs a KDE prefix as it's a KDE specific extension |
| src/netwm_def.h | ||
| 502 ↗ | (On Diff #31307) | You cannot change enum values. Please add the new one afterwards |
| src/platforms/xcb/atoms_p.h | ||
| 136 ↗ | (On Diff #31307) | Also a KDE prefix is needed |
| src/platforms/xcb/netwm.cpp | ||
| 26 ↗ | (On Diff #31307) | Unrelated change |
Just checked:
An implementation MAY add new atoms to this list. Implementations without extensions MUST ignore any unknown atoms, effectively removing them from the list. These extension atoms MUST NOT start with the prefix _NET.
So all is fine, but the prefix needs to change
An implementation MAY add new atoms to this list. Implementations without extensions MUST ignore any unknown atoms, effectively removing them from the list. These extension atoms MUST NOT start with the prefix _NET.
Thanks, Martin. That's very interesting. I'm in the process of making the changes. Does the invalid atom name mean KWin would not honor it? Or does it "just" mean it's out of compliance with a standard?
Thanks, @graesslin. I enjoyed this challenge.
I removed the [WIP] tag from the title.
Martin, @sharvey does not yet have commit access. Would you like to land this patch for him, or should I?
The CI for kwindowsystem fails ever since this was committed, can somebody please take a look?
FAIL! : KWindowInfoX11Test::testState(skipSwitcher) Compared values are not the same
Loc: [/home/jenkins/workspace/Frameworks kwindowsystem kf5-qt5 SUSEQt5.10/autotests/kwindowinfox11test.cpp(230)]
Ping? CI is still broken by this skipSwitcher stuff.
FAIL! : KWindowInfoX11Test::testState(skipSwitcher) Compared values are not the same Actual (int(info3.state())): 0 Expected (int(state)) : 4096 Loc: [/home/jenkins/workspace/Frameworks/kwindowsystem/kf5-qt5 SUSEQt5.10/autotests/kwindowinfox11test.cpp(230)]