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 122086 - Pager and ewmh move viewport
Pager and ewmh move viewport
Status: RESOLVED FIXED
Product: libwnck
Classification: Core
Component: general
2.4.x
Other Linux
: Normal normal
: ---
Assigned To: libwnck maintainers
libwnck maintainers
: 122512 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-09-12 08:13 UTC by olivier.chapuis
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Pointer location -> regular viewport origin. (2.88 KB, patch)
2003-09-23 21:48 UTC, Kim Woelders
none Details | Review
Cleaned up version of previous patch (1.90 KB, patch)
2003-11-02 09:47 UTC, Kim Woelders
none Details | Review

Description olivier.chapuis 2003-09-12 08:13:59 UTC
libwnck pager does not support large desktop (see the EWMH spec for
the terminology). So I do not understand why in src/pager.c
(wnck_pager_button_release) wnck_screen_move_viewport is called.

Without this call the libwnck pager in "Workspace name mode"
works fine with a large desktop (just a desktop switcher).
 
Please, remove this call. This does not break the pager and
metacity interaction.

PS1: if WNCK_PAGER_DISPLAY_CONTENT the libwnck pager is totally broken
with large desktop. This is **not** the point of this report.

PS2: Even with large desktop support, for usability reason I do not
think that the button 1 click should move the view port at "mouse
click position" it should move to one of the large desktop page. Maybe
button 1 + a modifier can move the view port at "mouse click
position".
Comment 1 Christian Krause 2003-09-17 05:27:02 UTC
This bug triggers #122205 in sawfish (Random Window Positioning with
Gnome's Workspace Switcher). But I'm unsure if it's a bug of libwnck,
sawfish or both...
Comment 2 olivier.chapuis 2003-09-17 19:53:57 UTC
Dear Christian,

Why do you think it is a swafish bug? Firts of all, The problem occur
with all the window managers with EWMH support and with large desktop.
Secondly it is libwnck which send the move view port msg. The wm just
respect the EWMH specification. Finnaly, do not wory, removing the
wnck_screen_move_viewport call will break nothing when the desktop
has the size of the screen (for metacity or whatever wm configured
for matching this condition).

Regards, Olivier
Comment 3 Christian Krause 2003-09-18 18:09:52 UTC
Hi Olivier,

it was just a guess, that this is sawfish's bug too, because the EWMH
says in "Pagers":

"On a large desktop, the pager may offer a way to move the viewport."

IMHO this means that a window manager must be aware of this option.

But I really don't know it exactly. :-(


regards,
christian
Comment 4 Havoc Pennington 2003-09-18 22:26:27 UTC
Kim Woelders wrote the viewport support, I haven't ever tried it.
Kim can you comment on this issue?
Comment 5 Kim Woelders 2003-09-19 07:10:10 UTC
First, libwnck does support large desktops, and IMO a pager click on a
large desktop should send a change viewport message.

Currently, a pager click sends _NET_DESKTOP_VIEWPORT with the pointer
location transformed to the corresponding location on the large desktop.

I don't think that the spec says exactly how this mechanism should be
implemented, so I have imagined that it should work as follows:
- The pager sends the pointer click location in large desktop coordinates.
- The WM uses that location according to its design.

Some WM's (e.g. enlightenment-0.16.6-preX) will move the viewport in
steps of the screen size so that the pointer location will become
on-screen.
Other WM's (?) might want to center the screen on the location.
WM's without large desktop support should (obviously?) simply ignore it.

One solution that would probably fix problems with WM's not supporting
large desktops but not ignoring _NET_DESKTOP_VIEWPORT could be to call
wnck_screen_move_viewport() only if wnck_workspace_is_virtual() is
true (this might actually be a good idea in any case).

Please do not remove the call to wnck_screen_move_viewport as it would
break the gnome pager/enlightenment interaction.

PS)
What does the "easy-fix" keyword mean?
Comment 6 Havoc Pennington 2003-09-19 15:29:41 UTC
easy-fix is supposed to make the bug show up on a list of things
developers just getting started with GNOME might want to try fixing.
Comment 7 olivier.chapuis 2003-09-23 06:17:49 UTC
kim wrote:
> I don't think that the spec says exactly how this mechanism should be
> implemented, so I have imagined that it should work as follows:
>- The pager sends the pointer click location in large desktop
> coordinates.
> - The WM uses that location according to its design.

The problem with this logic is that you limit the possibility
of an "ewmh pager". Typically a pager may want to ask to move
the view port

- at pointer click location
- or so that the pointer click is at the center of the screen
- or at the closest page of the large desktop
- or whatever other logic

If the wm should interpret the _NET_DESKTOP_VIEWPORT msg you
fallback in one possible behavior. I think that the wm should
respect strictly the spec: ignore _NET_DESKTOP_VIEWPORT or
strictly respect this msg.
So, what I claimed is that for _usability_ reason the best behavior
is to move at the closest page. But of course this does not make
sense for the gnome pager at it does not support large desktop!!
Really it is funny that gnome pager does not support large desktop
and send _NET_DESKTOP_VIEWPORT msgs (a the gnome pager looks really
strange after such msg is procceded).

> One solution that would probably fix problems with WM's not
> supporting large desktops but not ignoring _NET_DESKTOP_VIEWPORT
> could be to call wnck_screen_move_viewport() only if
> wnck_workspace_is_virtual() is true (this might actually be a
> good idea in any case).

Yes, I think that this is a very good idea in *_any_* case.

Regards, Olivier
Comment 8 Kim Woelders 2003-09-23 21:42:03 UTC
> But of course this does not make sense for the gnome pager at it
> does not support large desktop!! 

Why do you keep saying that?!?
From my point of view, libwnck (including pager) supports large
desktops perfectly well with the enlightenment WM.
I don't claim that large desktop support is perfect but I do claim
there is an attempt to implement it, and I suggest that if there are
problems in connection with other WM's (e.g. FVWM?), we figure out
what the problems are and fix whatever is broken.
If you don't think libwnck supports large desktops, please explain 
why.

As for the _NET_DESKTOP_VIEWPORT client message:
Yes, reporting the pointer location was more of a quick hack than a
proper implementation. I now suggest to let the location sent with
the _NET_DESKTOP_VIEWPORT message be the origin of the "regular" 
viewport (screen size stepped) containing the pointer.
Patch about to be attached.
An alternative implementation (center on pointer) is included.

The patch also
- avoids sending the desktop switch message if we click on the current 
  desk.
- avoids sending the viewport change message if there is no change.
The latter avoids sending any _NET_DESKTOP_VIEWPORT messages if
the WM doesn't support large desktops.
Comment 9 Kim Woelders 2003-09-23 21:48:20 UTC
Created attachment 20227 [details] [review]
Pointer location -> regular viewport origin.
Comment 10 olivier.chapuis 2003-09-25 07:59:35 UTC
> If you don't think libwnck supports large desktops, please explain 
> why.

I am sorry, the support is ok (I was confused).

I think the patch you send improve/fix the usability of the
gnome pager.

Thanks, Olivier
Comment 11 Christian Krause 2003-10-07 21:23:32 UTC
Hi,

I can confirm that Kim's patch solves the problem #122205 .
It would be nice if the patch could be applied and the bug closed.

regards,
christian
Comment 12 Vincent Untz 2003-10-16 15:00:26 UTC
adding PATCH keyword
Comment 13 Sven Neumann 2003-10-19 17:06:03 UTC
IMO the priority of this bug should be raised since it severily hurts
useability of the workspace switcher applet. I would even say the
applet becomes completely unuseable. This bug is a major PITA and
deserves a severity level of major or critical.
Comment 14 Sebastian Kapfer 2003-10-21 12:25:49 UTC
*** Bug 124086 has been marked as a duplicate of this bug. ***
Comment 15 Havoc Pennington 2003-10-28 01:16:02 UTC
Feel free to commit this patch, sorry I lost track of it before.
Though if you're sure the new code is right you might get rid of the
#ifdef stuff.
Comment 16 Kim Woelders 2003-11-02 09:47:27 UTC
Created attachment 21124 [details] [review]
Cleaned up version of previous patch
Comment 17 Kim Woelders 2003-11-02 09:54:01 UTC
The last patch is identical to the original one except that the
ifdef'ed stuff has been taken out as suggested by HP.

Is somebody going to check it in?
Comment 18 Kim Woelders 2003-12-07 09:01:04 UTC
Ping?
I suggest (again) to apply the last patch and close the bug.
Comment 19 Mark McLoughlin 2004-04-14 14:55:49 UTC
Sorry about the delay Kim. Committed now:

2004-04-14  Mark McLoughlin  <mark@skynet.ie>
                                                                               
                                                               
        Patch from Kim Woelders in bug #122086.
                                                                               
                                                               
        * libwnck/pager.c: (wnck_pager_button_release): only switch
        workspaces if actually clicking on a different workspace
        and move to viewport co-ordinates (0, 0) rather then where
        the mouse actually clicks.
                                                                               
                                                               
Comment 20 Mark McLoughlin 2004-04-19 15:06:12 UTC
*** Bug 122512 has been marked as a duplicate of this bug. ***