On Sun, Oct 2, 2011 at 1:02 PM, Edward K. Ream <[email protected]> wrote:

> Still, the more I think of using idle time, the less I like it.  I'm
> going to attempt to fix the bug by saving and restoring the body
> pane's scrollbars in k.showStateAndMode, which I think is the real
> source of the problem.

This has turned out to be one of the thorniest bugs I've ever
encountered.  Rev 4515.1.6 (see the children of rev 4517) contains the
result of two intense days of work.

Here are some notes, based on the checkin log.

1.  The fix is good, but not perfect.  In some cases, mouse clicks can
still cause unwanted scrolling at the end of long body text, but the
fixes ensure that even when this happens the cursor is in fact
visible.

I'm not sure whether the unwanted scrolling is because I am
overlooking some of Leo's code (it's surprisingly difficult to
determine all the event-related code that gets executed) or because of
glitches, if not outright bugs, in Qt itself.

I am beginning to suspect the latter: calls to
QTextBrowser.ensureCursorVisibible() apparently do too much: these
calls can scroll the text even when the cursor is *already* visible.
But to repeat: I can't complain about this for sure: there is just way
too much that is going on behind the scenes.

2. The new code abandons the attempt to execute code at idle time
using g.app.gui.runAtIdle.  I'm not sure runAtIdle works at all.  In
any event, I distrust using idle-time code here...

3.  It would be possible (I think) to eliminate all the scrolling
problems simply by eliminating the call to k.keyboardQuit in the
over-ridden mouseReleaseEvent method.  However, this would not be a
good idea--the user will expect that clicking anywhere outside the
minibuffer will terminate the minibuffer state.  Thus, the call to
k.keyboardQuit is essential.

4.  This is fantastically difficult code to understand and to get
working.  The final results are just three or four calls to
w.seeInsertPoint (for high-level w) or w.ensureCursorVisible (for
low-level w).  But I have tried literally hundreds of alternatives.

The breakthrough came when I abandoned the idea of saving and
restoring vertical scrolling position.  This simply never works: I
have no idea why.  Instead, w.seeInsertPoint or w.ensureCursorVisible
work.  One would naively expect that either approach would be pretty
much equivalent, but not so.

In short, the new code fixes the most egregious scrolling problems,
and does so in a simple, relatively-easy-to-understand way. The
remaining problems have the smell of a Qt problem, but I can't be sure
of that.

I've done all the work so far on Windows 7.  I'll play with the code
on Ubuntu next.

Edward

P.S. Here is the checkin log:

QQQQ
An almost complete fix for the scrolling problems encountered at the
end of long body text. Clicks can still cause lesser problems.

We could eliminate mouse-click problems by omitting the call to
k.keyboardQuit in the mouse-release handler, but that would cause
mouse-clicks not to reset the key state, and that would be confusing.

The new code is much better than before, and I don't know how to
improve it further.

The Aha is that trying to save & restore scrollbar state does not
work: don't ask me why.
Instead, the fix is to call either w.seeInsertPoint (for high-level w)
or w.ensureCursorVisible (for low-level w).

All unit tests pass, which is significant: some seemingly innocent
changes cause many tests to fail.
QQQQQ

EKR

-- 
You received this message because you are subscribed to the Google Groups 
"leo-editor" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/leo-editor?hl=en.

Reply via email to