468167 - deleting selected top of document (from below) no longer scrolls what remains into view

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[David Baron :dbaron:]

When selecting multiple pages at the top of the document, such that it's the bottom end of the selection that's visible, and then deleting that part, we no longer scroll the remaining (short) part of the document into view.

(There are probably some other similar steps to reproduce, but I'm keeping the description specific for now.  It does seem to be a requirement that the amount of stuff left in the textarea is not enough to require scrollbars, though.)

Steps to reproduce:
 * load https://bugzilla.mozilla.org/attachment.cgi?id=351052&action=edit
 * click "Edit attachment as Comment"
 * place the mouse cursor at the very beginning of the textfield
 * hold down the Shift key and press PgDn three times, and then the down arrow to get down to the "case NS_THEME_CHECKBOX" line
 * press backspace to delete the selection

Expected results: remaining part of the field is entirely visible

Actual results: field appears blank, but things become visible again if you hit the down arrow key

This regressed between Linux nightlies 2008-12-04-02-mozilla-central and 2008-12-05-02-mozilla-central.

Comment 1

[David Baron :dbaron:]

My first guess was http://hg.mozilla.org/mozilla-central/rev/f7e30552f3a7 but that does not seem to be the cause.

Comment 2

[David Baron :dbaron:]

This was caused by http://hg.mozilla.org/mozilla-central/rev/cfc553938038 .

Comment 3

[David Baron :dbaron:]

Attachment: a mochitest for this bug

(Needs to be dropped into objdir/_tests/testing/mochitest/tests/ somewhere to be run... or eventually put into a patch.)

Comment 4

[David Baron :dbaron:]

Attachment: a mochitest for this bug

Oops, don't cross keyCode and charCode, which prevents a weird character from showing up.  (I'm surprised it worked at all.)

Comment 5

[Markus Stange [:mstange]]

This is the change that caused this bug:

--- a/layout/generic/nsGfxScrollFrame.cpp	Thu Dec 04 14:17:48 2008 +0100
+++ b/layout/generic/nsGfxScrollFrame.cpp	Tue Dec 02 14:18:08 2008 +0100
@@ -1910,7 +1910,8 @@ void nsGfxScrollFrameInner::CurPosAttrib
       InternalScrollPositionDidChange(curPosX, curPosY);
       mFrameInitiatedScroll = PR_FALSE;
     }
-    ScrollbarChanged(mOuter->PresContext(), x, y, isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : 0);
+    ScrollbarChanged(mOuter->PresContext(), x, y,
+                     isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : NS_VMREFRESH_DEFERRED);
   }
 }

It wasn't necessary in the first place, so I just backed it out:
http://hg.mozilla.org/mozilla-central/rev/e8c07223b324

Then I added your test (after double-checking that it works):
http://hg.mozilla.org/mozilla-central/rev/28c7880df205

Thanks for the test!

Comment 6

[Markus Stange [:mstange]]

I didn't track down the real cause of this bug; I suspect that some part of the codepath made the assumption that ScrollTo is synchronous when smooth scrolling isn't used (which was no longer true after bug 463042). The fact that nsGfxScrollFrameInner::CurPosAttributeChanged does extra work when using smooth scrolling supports this theory.

Comment 7

[Serge Gautherie (:sgautherie)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/18ddfb10612a

Comment 8

[Aakash Desai [:aakashd]]

Seeing that there haven't been any duplicate sightings for this issue for 4
months, I'm going to go ahead and verify this unless someone has an issue with
that.