Closed Bug 271161 Opened 20 years ago Closed 17 years ago

Bad/broken WM_SIZE_HINTS [Gtk1.2.10]

Categories

(Core Graveyard :: GFX: Gtk, defect)

x86
Linux
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: flobber, Assigned: blizzard)

Details

User-Agent:       Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.7.5) Gecko/20041120 Firefox/1.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.7.5) Gecko/20041120 Firefox/1.0

Firefox doesn't sets WM_SIZE_HINTS properly.
The browsers top-level windows gets i.e.:

    WM_NORMAL_HINTS(WM_SIZE_HINTS):
                program specified size: 200 by 200

(seems to be the Gtk default - the actual window is
much larger), the preferences window gets:

    WM_NORMAL_HINTS(WM_SIZE_HINTS):
                program specified location: 0, 0

What actually happens on-screen depends heavily on
the used window manager (wrong or manual placement,
wrong sizes, ...).  I.e. with fvwm, transient-for
windows are placed automatically at the previous
position (but at 0/0 the first time) whereas the
browser window is never placed automatically but
has the right size.

The (IMHO) right handling would be: always set
the psize to the wanted values and the pposition
to the last used position.  Don't set pposition
when the window is shown the first time.

I had a look at gtk/nsWindow.cpp and found some
calls to gdk_widget_set_uposition but I've yet to
find a Firefox window with a uposition.  Somehow
the logic there seems to be broken...

Reproducible: Always
Steps to Reproduce:
Start Firefox and run xprop on top-level windows.
Assignee: bugs → blizzard
Component: OS Integration → GFX: Gtk
Product: Firefox → Core
QA Contact: firefox.os-integration → ian
Version: unspecified → Trunk
Found the culprit.  It's this ifdef in nsXULWindow.cpp

|#if defined(XP_UNIX) && !defined(XP_MACOSX)
|    // don't override WM placement on unix for independent, top-level windows
|    // (however, we think the benefits of intelligent dependent window placemen
|    // trump that override.)
|    if (!parentWindow)
|      positionSet = PR_FALSE;
|#endif

which prohibits calling the low-level function which sets
the window position.

Furthermore, the low-level function (widget/src/gtk/nsWindow.cpp
nsWindow::Move) uses gtk_widget_set_uposition which has an
implementation bug - it always sets the PPosition to 0/0.
An additional call to gdk_window_set_hints fixes that.

And at last, nsXULWindow::StaggerPosition is window manager
policy under Unix/X11.  Should be disabled.

A complete diff for those three changes:

--- mozilla-orig/xpfe/appshell/src/nsXULWindow.cpp      Mon Oct  4 20:21:21 2004
+++ mozilla/xpfe/appshell/src/nsXULWindow.cpp   Wed Nov 24 22:55:09 2004
@@ -943,17 +943,8 @@ void nsXULWindow::OnChromeLoaded()
         markupViewer->SizeToContent();
     }
 
-    PRBool positionSet = PR_TRUE;
     nsCOMPtr<nsIXULWindow> parentWindow(do_QueryReferent(mParentWindow));
-#if defined(XP_UNIX) && !defined(XP_MACOSX)
-    // don't override WM placement on unix for independent, top-level windows
-    // (however, we think the benefits of intelligent dependent window placemen
t
-    // trump that override.)
-    if (!parentWindow)
-      positionSet = PR_FALSE;
-#endif
-    if (positionSet)
-      positionSet = LoadPositionFromXUL();
+    PRBool positionSet = LoadPositionFromXUL();
     LoadMiscPersistentAttributesFromXUL();
 
     //LoadContentAreas();
@@ -1197,6 +1188,7 @@ PRBool nsXULWindow::LoadMiscPersistentAt
 void nsXULWindow::StaggerPosition(PRInt32 &aRequestedX, PRInt32 &aRequestedY,
                                   PRInt32 aSpecWidth, PRInt32 aSpecHeight)
 {
+#if defined(XP_MACOSX) || !defined(XP_UNIX)  //ET: this is wm's policy
   const PRInt32 kOffset = 22; // XXX traditionally different for mac.
   const PRInt32 kSlop = 4;
   nsresult rv;
@@ -1306,6 +1298,7 @@ void nsXULWindow::StaggerPosition(PRInt3
       }
     } while(1);
   } while (keepTrying);
+#endif
 }
 
 NS_IMETHODIMP nsXULWindow::LoadTitleFromXUL()
--- mozilla-orig/widget/src/gtk/nsWindow.cpp    Sat Feb 14 01:22:15 2004
+++ mozilla/widget/src/gtk/nsWindow.cpp Wed Nov 24 16:57:52 2004
@@ -2693,10 +2693,14 @@ NS_IMETHODIMP nsWindow::Move(PRInt32 aX,
       oldrect.x = aX;
       oldrect.y = aY;
       mParent->WidgetToScreen(oldrect, newrect);
-      gtk_widget_set_uposition(mShell, newrect.x, newrect.y);
-    } else {
-      gtk_widget_set_uposition(mShell, aX, aY);
+      aX = newrect.x;
+      aY = newrect.y;
     }
+    gtk_widget_set_uposition(mShell, aX, aY);
+    /*ET: bug in gtk_widget_set_uposition sets WM_SIZE_HINTS.PPos to 0/0 */
+    /*force it to the real value*/
+    if (!GTK_WIDGET_MAPPED(mShell))  
+      gdk_window_set_hints(mShell->window, aX, aY, 0,0,0,0, GDK_HINT_POS);
   }
   else if (mSuperWin) {
     gdk_window_move(mSuperWin->shell_window, aX, aY);

Ciao.
Severity: normal → minor
Summary: Bad/broken WM_SIZE_HINTS [Gtk1.2.9] → Bad/broken WM_SIZE_HINTS [Gtk1.2.10]
Reporter, could you attach the patch as an attachment using
https://bugzilla.mozilla.org/attachment.cgi?bugid=271161&action=enter ?

What's the actual effect of this change on allowing windowmanagers to place
windows?  The "don't override WM placement on unix" comment is there for a
reason, last I checked...
Status: UNCONFIRMED → NEW
Ever confirmed: true
> Reporter, could you attach the patch as an attachment
> using [...]

Hmm... seems there's another bug in Firefox.  I can try
as often as I want, Bugzilla just says:

  "You did not specify a file to attach"

Sorry.

> What's the actual effect of this change on allowing
> windowmanagers to place windows?

Ok, some background about X11 window placement policy:

  When a top level window has no WM_SIZE_HINTS attached
it's completely the window manager's task to place it.
The WM may decide to let you place the window by hand
(manual placement) or he may select some empty space
for automatic placement of he may "stagger" the windows
with others of the same type or whatever suits him.
Most WMs have a couple of preferences to select different
behaviour.

  The WM_SIZE_HINTS give the window manager some hints
on how to handle a window.  For the position there
are two flags that define how the window position
should be treated: PPosition (program specified
position) and USPosition (user specified position).

  Setting PPosition tells the WM that x/y was chosen
by the application by some means.  It's a suggestion
for a good position.  The WM may ignore it.  Often
WMs have an option to turn PPos honouring on/off
(due to a couple of apps with broken PPos handling).

  Setting USPosition tells the WM that x/y was chosen
by the user (i.e. via a -geometry command line option).
It is highly recommendet that the WM honours this
position as the user has explicitely asked for this
position.

To complicate it even further, there are two places
for storing the x/y values: in the hints structure
and in the window itself.  It is recommended to set
both places to the same values.  WMs differ about
what place to actually use.

> The "don't override WM placement on unix" comment
> is there for a reason, last I checked...

Well, I guess the author had a WM that was confused
by the bug in GTK.  GTK was never specially good with
WM-interaction, i.e. there's no function to set the
USPosition flag, only PPosition.  Furthermore, it
always sets the x/y fields in the hints structure
to 0/0.  If your WM honours the PPos and takes the
position from the hints structure all of your windows
will be placed at 0/0 - very annoying.

The original author's solution: disable the whole "set
position of top-level windows" code.

The patch works around the GTK bug by setting the x/y
fields in the hints structure to the same values as
those in the window itself.  xprop now reports for
example:

  WM_NORMAL_HINTS(WM_SIZE_HINTS):
                program specified location: 766, 46

With that in place, the original positioning code (used
on all other architectures) can be reenabled.  It gives
a sane result: windows without a saved position get no
WM_SIZE_HINTS (WM decides how to handle them) and windows
with a saved position get WM_SIZE_HINTS with PPosition set.

Ciao.
I fixed this in gtk2 some time ago.  Probably by making sure that the position
was never set if it was never explicitly set from XUL.
Gtk1/xlib widget code has been removed from the tree and comment 4 indicates
this bug was fixed for gtk2.  This bug isn't a branch candidate IMO.

-> WONTFIX
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.