* xselect.c (x_catch_errors_unwind): New function.

	(x_reply_selection_request): Put x_uncatch_errors in an unwind.
	(Fx_get_atom_name): Call x_uncatch_errors earlier.

	* window.c (Qscroll_up, Qscroll_down): New syms.
	(window_scroll_pixel_based): Make preserve_y static to avoid
	getting point stuck when scrolling 1 line.

3 files changed, 63 insertions, 29 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 79db6d09d0b..faba78efd63 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,13 @@
+2006-02-28  Chong Yidong  <cyd@stupidchicken.com>
+
+        * xselect.c (x_catch_errors_unwind): New function.
+        (x_reply_selection_request): Put x_uncatch_errors in an unwind.
+        (Fx_get_atom_name): Call x_uncatch_errors earlier.
+
+        * window.c (Qscroll_up, Qscroll_down): New syms.
+        (window_scroll_pixel_based): Make preserve_y static to avoid
+        getting point stuck when scrolling 1 line.
+
 2006-02-26  Chong Yidong  <cyd@stupidchicken.com>
 
         * xterm.h, xterm.c (x_uncatch_errors): Delete unneccessary
diff --git a/src/window.c b/src/window.c
index 41eaf78bea0..cd7c1e6a625 100644
--- a/src/window.c
+++ b/src/window.c
@@ -50,6 +50,7 @@ Boston, MA 02110-1301, USA.  */
 
 
 Lisp_Object Qwindowp, Qwindow_live_p, Qwindow_configuration_p;
+Lisp_Object Qscroll_up, Qscroll_down;
 Lisp_Object Qwindow_size_fixed;
 extern Lisp_Object Qleft_margin, Qright_margin;
 
@@ -4721,9 +4722,9 @@ window_scroll_pixel_based (window, n, whole, noerror)
   struct text_pos start;
   Lisp_Object tem;
   int this_scroll_margin;
-  int preserve_y;
   /* True if we fiddled the window vscroll field without really scrolling.   */
   int vscrolled = 0;
+  static int preserve_y = -1;
 
   SET_TEXT_POS_FROM_MARKER (start, w->start);
 
@@ -4787,9 +4788,18 @@ window_scroll_pixel_based (window, n, whole, noerror)
      point in the same window line as it is now, so get that line.  */
   if (!NILP (Vscroll_preserve_screen_position))
     {
-      start_display (&it, w, start);
-      move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
-      preserve_y = it.current_y;
+      /* We preserve the goal pixel coordinate across consecutive
+         calls to scroll-up or scroll-down.  This avoids the
+         possibility of point becoming "stuck" on a tall line when
+         scrolling by one line.  */
+      if (preserve_y < 0
+          || (current_kboard->Vlast_command != Qscroll_up
+              && current_kboard->Vlast_command != Qscroll_down))
+        {
+          start_display (&it, w, start);
+          move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
+          preserve_y = it.current_y;
+        }
     }
   else
     preserve_y = -1;
@@ -4926,10 +4936,9 @@ window_scroll_pixel_based (window, n, whole, noerror)
         {
           /* If we have a header line, take account of it.
              This is necessary because we set it.current_y to 0, above.  */
-          if (WINDOW_WANTS_HEADER_LINE_P (w))
-            preserve_y -= CURRENT_HEADER_LINE_HEIGHT (w);
-
-          move_it_to (&it, -1, -1, preserve_y, -1, MOVE_TO_Y);
+          move_it_to (&it, -1, -1,
+                      preserve_y - (WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0 ),
+                      -1, MOVE_TO_Y);
           SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
         }
       else
@@ -4983,15 +4992,9 @@ window_scroll_pixel_based (window, n, whole, noerror)
         {
           SET_TEXT_POS_FROM_MARKER (start, w->start);
           start_display (&it, w, start);
-#if 0  /* It's wrong to subtract this here
-          because we called start_display again
-          and did not alter it.current_y this time.  */
-
-          /* If we have a header line, take account of it.  */
-          if (WINDOW_WANTS_HEADER_LINE_P (w))
-            preserve_y -= CURRENT_HEADER_LINE_HEIGHT (w);
-#endif
-
+          /* It would be wrong to subtract CURRENT_HEADER_LINE_HEIGHT
+             here because we called start_display again and did not
+             alter it.current_y this time.  */
           move_it_to (&it, -1, -1, preserve_y, -1, MOVE_TO_Y);
           SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
         }
@@ -6988,6 +6991,12 @@ init_window ()
 void
 syms_of_window ()
 {
+  Qscroll_up = intern ("scroll-up");
+  staticpro (&Qscroll_up);
+
+  Qscroll_down = intern ("scroll-down");
+  staticpro (&Qscroll_down);
+
   Qwindow_size_fixed = intern ("window-size-fixed");
   staticpro (&Qwindow_size_fixed);
   Fset (Qwindow_size_fixed, Qnil);
diff --git a/src/xselect.c b/src/xselect.c
index a0b4b091805..30739c74331 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -55,6 +55,7 @@ static void x_decline_selection_request P_ ((struct input_event *));
 static Lisp_Object x_selection_request_lisp_error P_ ((Lisp_Object));
 static Lisp_Object queue_selection_requests_unwind P_ ((Lisp_Object));
 static Lisp_Object some_frame_on_display P_ ((struct x_display_info *));
+static Lisp_Object x_catch_errors_unwind P_ ((Lisp_Object));
 static void x_reply_selection_request P_ ((struct input_event *, int,
                                            unsigned char *, int, Atom));
 static int waiting_for_other_props_on_window P_ ((Display *, Window));
@@ -611,6 +612,15 @@ x_selection_request_lisp_error (ignore)
     x_decline_selection_request (x_selection_current_request);
   return Qnil;
 }
+
+static Lisp_Object
+x_catch_errors_unwind (dummy)
+     Lisp_Object dummy;
+{
+  BLOCK_INPUT;
+  x_uncatch_errors ();
+  UNBLOCK_INPUT;
+}
 
 
 /* This stuff is so that INCR selections are reentrant (that is, so we can
@@ -703,8 +713,11 @@ x_reply_selection_request (event, format, data, size, type)
   if (reply.property == None)
     reply.property = reply.target;
 
-  /* #### XChangeProperty can generate BadAlloc, and we must handle it! */
   BLOCK_INPUT;
+  /* The protected block contains wait_for_property_change, which can
+     run random lisp code (process handlers) or signal.  Therefore, we
+     put the x_uncatch_errors call in an unwind.  */
+  record_unwind_protect (x_catch_errors_unwind, Qnil);
   x_catch_errors (display);
 
 #ifdef TRACE_SELECTION
@@ -858,9 +871,8 @@ x_reply_selection_request (event, format, data, size, type)
      UNBLOCK to enter the event loop and get possible errors delivered,
      and then BLOCK again because x_uncatch_errors requires it.  */
   BLOCK_INPUT;
-
+  /* This calls x_uncatch_errors.  */
   unbind_to (count, Qnil);
-  x_uncatch_errors ();
   UNBLOCK_INPUT;
 }
 
@@ -1370,7 +1382,7 @@ x_get_foreign_selection (selection_symbol, target_type, time_stamp)
   Atom selection_atom = symbol_to_x_atom (dpyinfo, display, selection_symbol);
   Atom type_atom;
   int secs, usecs;
-  int count;
+  int count = SPECPDL_INDEX ();
   Lisp_Object frame;
 
   if (CONSP (target_type))
@@ -1392,6 +1404,10 @@ x_get_foreign_selection (selection_symbol, target_type, time_stamp)
 
   BLOCK_INPUT;
 
+  /* The protected block contains wait_reading_process_output, which
+     can run random lisp code (process handlers) or signal.
+     Therefore, we put the x_uncatch_errors call in an unwind.  */
+  record_unwind_protect (x_catch_errors_unwind, Qnil);
   x_catch_errors (display);
 
   TRACE2 ("Get selection %s, type %s",
@@ -1409,8 +1425,6 @@ x_get_foreign_selection (selection_symbol, target_type, time_stamp)
 
   frame = some_frame_on_display (dpyinfo);
 
-  count = SPECPDL_INDEX ();
-
   /* If the display no longer has frames, we can't expect
      to get many more selection requests from it, so don't
      bother trying to queue them.  */
@@ -1432,9 +1446,10 @@ x_get_foreign_selection (selection_symbol, target_type, time_stamp)
   TRACE1 ("  Got event = %d", !NILP (XCAR (reading_selection_reply)));
 
   BLOCK_INPUT;
+  if (x_had_errors_p (display))
+    error ("Cannot get selection");
+  /* This calls x_uncatch_errors.  */
   unbind_to (count, Qnil);
-  x_check_errors (display, "Cannot get selection: %s");
-  x_uncatch_errors ();
   UNBLOCK_INPUT;
 
   if (NILP (XCAR (reading_selection_reply)))
@@ -2655,6 +2670,7 @@ If the value is 0 or the atom is not known, return the empty string.  */)
   Lisp_Object ret = Qnil;
   Display *dpy = FRAME_X_DISPLAY (f);
   Atom atom;
+  int had_errors;
 
   if (INTEGERP (value))
     atom = (Atom) XUINT (value);
@@ -2667,14 +2683,13 @@ If the value is 0 or the atom is not known, return the empty string.  */)
 
   BLOCK_INPUT;
   x_catch_errors (dpy);
-
   name = atom ? XGetAtomName (dpy, atom) : "";
+  had_errors = x_had_errors_p (dpy);
+  x_uncatch_errors ();
 
-  if (! x_had_errors_p (dpy))
+  if (!had_errors)
     ret = make_string (name, strlen (name));
 
-  x_uncatch_errors ();
-
   if (atom && name) XFree (name);
   if (NILP (ret)) ret = make_string ("", 0);