From: PatR <rankin@nethack.org>
Date: Sat, 13 Mar 2021 02:06:43 +0000 (-0800)
Subject: scrollbar handling by X11
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=160344feaa63b70d4fbd03a7c98dab32c4122f35;p=nethack

scrollbar handling by X11

Looking up scrollbars did not work as intended.  The code wanted an
ancestor widget that had both horizontal and vertical scrollbars,
but menus either have none or just vertical.  The lookup code found
some top level widget and returned bad data.
---

diff --git a/doc/fixes37.0 b/doc/fixes37.0
index 0accbe778..2c1e523b1 100644
--- a/doc/fixes37.0
+++ b/doc/fixes37.0
@@ -559,6 +559,8 @@ X11: if perm_invent is set in NETHACKOPTIONS or config file, start with the
 	persistent inventory window displayed
 X11: use visctrl(response) when X11_yn_function() echoes prompt+response in
 	message window
+X11: when trying to lookup scrollbars in order to handle scrolling via keys,
+	menu handling always ended up finding the top window
 X11+OSX: after the "bad Atom" fix (below), the persistent inventory window
 	crept downward every time it got updated
 
diff --git a/include/winX.h b/include/winX.h
index af1ea596e..1ecfaf681 100644
--- a/include/winX.h
+++ b/include/winX.h
@@ -332,7 +332,7 @@ extern void highlight_yn(boolean);
 extern void nh_XtPopup(Widget, int, Widget);
 extern void nh_XtPopdown(Widget);
 extern void win_X11_init(int);
-extern void find_scrollbars(Widget, Widget *, Widget *);
+extern void find_scrollbars(Widget, Widget, Widget *, Widget *);
 extern void nh_keyscroll(Widget, XEvent *, String *, Cardinal *);
 
 /* ### winmesg.c ### */
diff --git a/win/X11/winX.c b/win/X11/winX.c
index 2ce3ac5f6..f2f5a24fb 100644
--- a/win/X11/winX.c
+++ b/win/X11/winX.c
@@ -2682,14 +2682,24 @@ win_X11_init(int dir)
 }
 
 void
-find_scrollbars(Widget w, Widget *horiz, Widget *vert)
-{
+find_scrollbars(
+    Widget w,      /* widget of interest; scroll bars are probably attached
+                      to its parent or grandparent */
+    Widget last_w, /* if non-zero, don't search ancestory beyond this point */
+    Widget *horiz, /* output: horizontal scrollbar */
+    Widget *vert)  /* output: vertical scrollbar */
+{
+    *horiz = *vert = (Widget) 0;
+    /* for 3.6 this looked for an ancestor with both scrollbars but
+       menus might have only vertical */
     if (w) {
         do {
             *horiz = XtNameToWidget(w, "*horizontal");
             *vert = XtNameToWidget(w, "*vertical");
+            if (*horiz || *vert)
+                break;
             w = XtParent(w);
-        } while (!*horiz && !*vert && w);
+        } while (w && (!last_w || w != last_w));
     }
 }
 
@@ -2715,7 +2725,7 @@ nh_keyscroll(Widget viewport, XEvent *event, String *params,
 
     direction = atoi(params[0]);
 
-    find_scrollbars(viewport, &horiz_sb, &vert_sb);
+    find_scrollbars(viewport, (Widget) 0, &horiz_sb, &vert_sb);
 
 #define H_DELTA 0.25 /* distance of horiz shift */
     /* vert shift is half of curr distance */
diff --git a/win/X11/winmenu.c b/win/X11/winmenu.c
index 48d47163e..4eb127310 100644
--- a/win/X11/winmenu.c
+++ b/win/X11/winmenu.c
@@ -290,17 +290,17 @@ menu_key(Widget w, XEvent *event, String *params, Cardinal *num_params)
                 X11_nhbell();
             return;
         } else if (ch == MENU_FIRST_PAGE || ch == MENU_LAST_PAGE) {
-            Widget hbar = (Widget) 0, vbar = (Widget) 0;
+            Widget hbar, vbar;
             float top = (ch == MENU_FIRST_PAGE) ? 0.0 : 1.0;
 
-            find_scrollbars(wp->w, &hbar, &vbar);
+            find_scrollbars(wp->w, wp->popup, &hbar, &vbar);
             if (vbar)
                 XtCallCallbacks(vbar, XtNjumpProc, &top);
             return;
         } else if (ch == MENU_NEXT_PAGE || ch == MENU_PREVIOUS_PAGE) {
-            Widget hbar = (Widget) 0, vbar = (Widget) 0;
+            Widget hbar, vbar;
 
-            find_scrollbars(wp->w, &hbar, &vbar);
+            find_scrollbars(wp->w, wp->popup, &hbar, &vbar);
             if (vbar) {
                 float shown, top;
                 Arg arg[2];
@@ -309,8 +309,7 @@ menu_key(Widget w, XEvent *event, String *params, Cardinal *num_params)
                 XtSetArg(arg[1], nhStr(XtNtopOfThumb), &top);
                 XtGetValues(vbar, arg, TWO);
                 top += ((ch == MENU_NEXT_PAGE) ? shown : -shown);
-                if (vbar)
-                    XtCallCallbacks(vbar, XtNjumpProc, &top);
+                XtCallCallbacks(vbar, XtNjumpProc, &top);
             }
             return;
         } else if (index(menu_info->curr_menu.gacc, ch)) {
diff --git a/win/X11/winmisc.c b/win/X11/winmisc.c
index 7987dbcba..b67e85b67 100644
--- a/win/X11/winmisc.c
+++ b/win/X11/winmisc.c
@@ -1814,16 +1814,14 @@ ec_key(Widget w, XEvent *event, String *params, Cardinal *num_params)
         ec_active = FALSE;
         return;
     } else if (ch == MENU_FIRST_PAGE || ch == MENU_LAST_PAGE) {
-        hbar = vbar = (Widget) 0;
-        find_scrollbars(w, &hbar, &vbar);
+        find_scrollbars(w, (Widget) 0, &hbar, &vbar);
         if (vbar) {
             top = (ch == MENU_FIRST_PAGE) ? 0.0 : 1.0;
             XtCallCallbacks(vbar, XtNjumpProc, &top);
         }
         return;
     } else if (ch == MENU_NEXT_PAGE || ch == MENU_PREVIOUS_PAGE) {
-        hbar = vbar = (Widget) 0;
-        find_scrollbars(w, &hbar, &vbar);
+        find_scrollbars(w, (Widget) 0, &hbar, &vbar);
         if (vbar) {
             XtSetArg(arg[0], nhStr(XtNshown), &shown);
             XtSetArg(arg[1], nhStr(XtNtopOfThumb), &top);