After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 609272 - gdm doesn't set WINDOWPATH any more
gdm doesn't set WINDOWPATH any more
Status: RESOLVED FIXED
Product: gdm
Classification: Core
Component: general
2.28.x
Other All
: Normal normal
: ---
Assigned To: Brian Cameron
GDM maintainers
Depends on:
Blocks: 618510
 
 
Reported: 2010-02-07 21:35 UTC by Samuel Thibault
Modified: 2010-07-02 19:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch fixing problem (5.11 KB, patch)
2010-02-15 21:31 UTC, Brian Cameron
none Details | Review
updated patch (5.34 KB, patch)
2010-03-12 23:47 UTC, Brian Cameron
committed Details | Review
fix for 64bit (1003 bytes, patch)
2010-05-11 18:52 UTC, Julien Cristau
accepted-commit_now Details | Review

Description Samuel Thibault 2010-02-07 21:35:07 UTC
Hello,

In 2.19.3, I got the support of the WINDOWPATH environment variable
in, but with the 2.21 rewrite, that support got lost. The consequence
is that now that Ubuntu uses version 2.28 of gdm, WINDOWPATH is not
defined and thus braille support is just broken.

I have to say I don't feel up to dive once more into gdm's rewritten
code to add the support again.

Basically, once the server is started, the XFree86_VT property should
be read to get the VT number where the Xorg server is started. 2.20's
gdm_get_current_vtnum() function can probably be reused as it was. This
number should be appended to the current WINDOWPATH environment
variable if any, see 2.20's gdm_window_path() function. All clients
started by gdm should then have their WINDOWPATH environment variable
defined to the result.
Comment 1 Paul Cutler 2010-02-07 22:35:58 UTC
Changing to general, not a docs bug
Comment 2 Brian Cameron 2010-02-08 21:38:21 UTC
Note bug #443557, which discusses this feature being integrated into GDM before the rewrite.
Comment 3 Brian Cameron 2010-02-15 21:31:12 UTC
Created attachment 153868 [details] [review]
patch fixing problem


This patch just basically ports the code from GDM 2.19 to GDM 2.29.  To make things a bit more simple, the gdm-slave just sets the environment variable after calling XOpenDisplay and the code in gdm-welcome-session.c and gdm-session-direct.c just pass the environment variable along.
Comment 4 Samuel Thibault 2010-02-15 21:38:04 UTC
Does a daemon always just run one display at a time?  If yes, then ok,
the patch looks fine.  If no, the problem is that you most probably
have a different VT number for each X server.
Comment 5 Brian Cameron 2010-02-15 22:45:11 UTC
GDM  has a main daemon which manages all displays, but the setting of WINDOWPATH in this patch is in the slave daemon, which runs per-display.  Each display has its own slave daemon.  So this should be fine.
Comment 6 Brian Cameron 2010-03-12 23:47:22 UTC
Created attachment 156025 [details] [review]
updated patch

In talking with Ray Strode on IRC, he recommended the patch be changed as follows:

- A better function name.  Now uses gdm_slave_set_windowpath instead of just
  gdm_slave_windowpath.
- The gdm_slave_set_windowpath function should only be called if the host is 
  local.  So the function call was moved into the block of code that is only
  called for local hosts.

Can this now go upstream?
Comment 7 Brian Cameron 2010-04-21 22:22:07 UTC
Fixed in master.
Comment 8 Julien Cristau 2010-05-11 18:52:09 UTC
Created attachment 160847 [details] [review]
fix for 64bit

The gdm_slave_set_windowpath function accesses the XFree86_VT property as an uint32_t if format is 32.  Unfortunately, Xlib stuffs 32bit properties in arrays of longs, which might be 64bit.  Attached patch should fix that.
Comment 9 Brian Cameron 2010-05-24 22:02:45 UTC
Thanks Julien, fixed in master.