Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement WM_STATE in X11 GUI agent #121

Merged
merged 2 commits into from
Dec 18, 2020
Merged

Conversation

bemoody
Copy link

@bemoody bemoody commented Dec 9, 2020

These changes implement the WM_STATE property on the X11 AppVM side - allowing applications to identify the toplevel windows of other applications that are running in the same VM.

This is necessary for "screen sharing" of individual windows in Firefox and possibly other, similar programs: QubesOS/qubes-issues#5789

@bemoody
Copy link
Author

bemoody commented Dec 9, 2020

V2: use the correct variable name (winid -> window), and added gpg signatures. Sorry I accidentally pushed a broken version the first time.

On a side note, I found that trying to build this using dpkg-buildpackage -b fails, because it expects the variable BACKEND_VMM to be defined. That variable presumably ought to be defined either in the project makefile or the debian/rules file.

Copy link
Member

@marmarek marmarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase it (and make the PR) on master branch. When merged, I'll cherry pick it to release4.0.
Otherwise looks good.

Benjamin Moody added 2 commits December 11, 2020 16:52
The internal variable "wm_state" is used to store the X atom
"_NET_WM_STATE".  Rename this variable to "net_wm_state" to avoid
confusion, because "WM_STATE" is something different from
"_NET_WM_STATE" and we need to handle both.
The WM_STATE property is defined by ICCCM, and should be set by the
window manager to indicate the current state of a client window.  It
is used for two primary purposes:

- A client can examine the property to determine whether the client's
  own window is conceptually "visible" (NormalState) or "hidden"
  (IconicState).

- A client can look for this property on other clients' windows, in
  order to distinguish application windows (which have the WM_STATE
  property) from windows created by the window manager.

One notable program that makes use of the latter feature is Mozilla
Firefox, which allows WebRTC sites to perform "screen sharing" - but
in order to share a single window rather than the whole screen, that
other window must have the WM_STATE property.

We want to set this property when a client window is mapped, and unset
it when the window is unmapped (withdrawn).  In Qubes, we pretend that
the window is always in normal state, regardless of whether it's
currently minimized/iconified/shaded in dom0.

Note that this property shouldn't be set for override-redirect
windows, since those are beyond the window manager's purview.
@bemoody bemoody changed the base branch from release4.0 to master December 11, 2020 22:04
@bemoody
Copy link
Author

bemoody commented Dec 11, 2020

Now rebased and I think I set the target branch correctly.

I haven't tested this on 4.1, but it ought to work. :)

@DemiMarie
Copy link
Contributor

DemiMarie commented Dec 17, 2020

@bemoody have you uploaded your key to a keyserver? If so, the pipeline probably just needs to be retried.

@marmarek
Copy link
Member

PipelineRetry

@marmarek
Copy link
Member

marmarek commented Dec 17, 2020

(the above is about gitlab-ci, not signature check - the key is still missing)

@bemoody
Copy link
Author

bemoody commented Dec 17, 2020 via email

@bemoody
Copy link
Author

bemoody commented Dec 17, 2020 via email

@DemiMarie
Copy link
Contributor

Using a subkey is considered best practice, and it should not confuse the verification scripts. If it does, that’s a bug in the scripts. @marmarek have the scripts been tested with a subkey?

@marmarek
Copy link
Member

My public key ID is FBE2DC9A105D63ABCC5E679D323A63063AC869DF and it should be on the keyservers

$ gpg --keyserver pool.sks-keyservers.net  --recv-key FBE2DC9A105D63ABCC5E679D323A63063AC869DF
gpg: keyserver receive failed: No data

Which one?

@marmarek
Copy link
Member

PipelineRetry

@DemiMarie
Copy link
Contributor

My public key ID is FBE2DC9A105D63ABCC5E679D323A63063AC869DF and it should be on the keyservers

$ gpg --keyserver pool.sks-keyservers.net  --recv-key FBE2DC9A105D63ABCC5E679D323A63063AC869DF
gpg: keyserver receive failed: No data

Which one?

gpg --recv-keys --keyserver=keys.openpgp.org -- FBE2DC9A105D63ABCC5E679D323A63063AC869DF

works.

Copy link
Member

@marmarek marmarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It works!

@marmarek marmarek merged commit a01c640 into QubesOS:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants