Re: [Emacs-diffs] emacs-26 f274cbd: Avoid reordering of output in 'shr-insert-document'

[Eli Zaretskii]

> From: Stefan Monnier <address@hidden>
> Cc: address@hidden
> Date: Sat, 16 Dec 2017 17:23:02 -0500
> 
> >> Is it possible to fix the problem in shr-pixel-column (since, according
> >> to the comment above, the problems comes from there)?
> > No, because the value of point to preserve is from the buffer which
> > was current before with-temp-buffer.
> 
> But when we enter shr-pixel-column, that buffer's point is still the
> right one, and when we leave shr-pixel-column it isn't any more, so
> somehow shr-pixel-column manages to "find" that buffer in order to
> modify it and hence we should also be able to similarly find that buffer
> and restore its point.
> 
> I mean, from a purely theoretical point of view, it *can* be
> solved there.  What I don't understand is where practical aspects end up
> getting it the way.

Purely theoretically, you could always do something like

  (with-current-buffer (window-buffer)
    (point))

to get at the value of point at entry into shr-pixel-column, and then
a similar gork before exiting.  But is that a good idea?  It will slow
down shr-pixel-column, which is already one of the hottest spots in
shr.el's rendering.  I don't think Lars will want to talk to me after
that...

What actually happens here is that save-window-excursion gets run
afoul by the preceding with-temp-buffer, and ends up saving the wrong
value of point.  Specifically, save_window_save has this snippet:

      if (BUFFERP (w->contents))
        {
          /* Save w's value of point in the window configuration.  If w
             is the selected window, then get the value of point from
             the buffer; pointm is garbage in the selected window.  */
          if (EQ (window, selected_window))
            p->pointm = build_marker (XBUFFER (w->contents),
                                      BUF_PT (XBUFFER (w->contents)),
                                      BUF_PT_BYTE (XBUFFER (w->contents)));
          else
            p->pointm = Fcopy_marker (w->pointm, Qnil);

In the scenario we have in this case, it decides to use w->pointm,
whereas the correct value is in BUF_PT.

Maybe Martin will have an idea for how to fix the logic there,
although it already sounds quite fragile to me, enough so to be afraid
of making any changes there on the release branch.  But maybe for
master we could find a solution that is reliable enough.  Maybe.  On
the release branch, I see no cake left to eat.