Closed Bug 468167 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: dbaron, Assigned: mstange)

References

Details

(Keywords: regression, verified1.9.1)

Attachments

(1 file, 1 obsolete file)

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.
Flags: blocking1.9.2?
My first guess was http://hg.mozilla.org/mozilla-central/rev/f7e30552f3a7 but that does not seem to be the cause.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: PC → All
Attached file a mochitest for this bug (obsolete) —
(Needs to be dropped into objdir/_tests/testing/mochitest/tests/ somewhere to be run... or eventually put into a patch.)
Oops, don't cross keyCode and charCode, which prevents a weird character from showing up.  (I'm surprised it worked at all.)
Attachment #351630 - Attachment is obsolete: true
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!
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: blocking1.9.2?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
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.
Flags: in-testsuite+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/18ddfb10612a
Keywords: fixed1.9.1
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.