399410 - [FIX]composer text box doesn't scroll down when typing past end of last line

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.9a9pre) Gecko/2007100703 SeaMonkey/2.0a1pre  (trunk nightly)

100% reproducible.  

Type anything on the bottom line in the email composer text box and keep 
typing until you type past the right edge of the box, and then the text 
you type will be entered below the bottom of the text box (the box won't 
scroll up).  The email composer just seems to take no notice that you've
typed below the bottom of the text area.

Note that unlike bug 393723, this bug does not affect the text editor in 
SM browser windows.  The problem is not seen when typing into a text box
in a browser page, like the box into which I am now typing this comment.
It appears only in the email composer.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Reassigning to Boris per his request in bug 393723 comment 15.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Nelson, does the bottom part of the scrollbar (e.g. the bottom scrollarrow) end up extending below the status bar when this happens?  That is, does it look like the entire editing area is too tall for the window all of a sudden?

That's what I'm getting here, at least sometimes.  Will dig more tomorrow.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: small screen shot of composer window showing problem

Boris, 
I'm not sure of the answer to your question in bug 393723 comment 16,
so I thought I'd attach this image and let you decide for yourself.
(I think maybe the answer is: yes, something's wrong with that scroll bar)

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, that's what I see.  This regressed between 2007-09-20-01 and 2007-09-21-01.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Looks like a regression from bug 396587.  Digging into it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So the key is this sequence of events:

nsSubDocumentFrame::ReflowFinished
PresShell::HandlePostedReflowCallbacks
PresShell::DidDoReflow
PresShell::ProcessReflowCommands
PresShell::DoFlushPendingNotifications
PresShell::FlushPendingNotifications
nsDocument::FlushPendingNotifications
nsDocShell::GetPositionAndSize
nsSubDocumentFrame::ReflowFinished

In particular, in ReflowFinished we set mPostedReflowCallback to false, then get our size, then get the docshell position and size, which triggers a layout flush that incidentally changes our size(!).  So we go through and set this new later size on the docshell... then unwind and set the old, earlier size.

roc, smaug, is it expected that reflow callback processing will reenter this way?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: This fixes it, even in the face of reentry.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 284573 [details] [diff] [review]
This fixes it, even in the face of reentry.

I think this is a very safe fix, and it fixes some nasty issues in mailnews and possibly with general scrollable iframe sizing.  Worth taking for the beta, in my opinion.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 284573 [details] [diff] [review]
This fixes it, even in the face of reentry.

a=drivers for after M9 freeze

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Need to handle destruction

This is identical to the previous patch, but with a weakframe check to handle |this| dying.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 287056 [details] [diff] [review]
Need to handle destruction

Do I want to know how GetPositionAndSize can kill the frame? Can it run script somehow?

Comment 12

[Boris Zbarsky [:bzbarsky]]

GetPositionAndSize flushes layout, which means it can run queued up style changes and XBL constructors....

Comment 13

[Boris Zbarsky [:bzbarsky]]

Checked in.