186066 - Typing in textareas causes redundant view resizing

Status: NEW

(Core :: Web Painting, defect)

Description

[Simon Fraser [no longer active]]

When typing in textareas (like this one), we invalidate the entire textarea for
each keypress.

Some investigation shows that this happens because of a couple of view resizes.

nsScrollBoxFrame::DoLayout calls Layout on its child box with the
NS_FRAME_NO_SIZE_VIEW flag (since we have a nsIBoxToBlockAdaptor in this case).
Down in that Layout call, nsBoxToBlockAdaptor::Reflow() calls
nsContainerFrame::FinishReflowChild() with flags NS_FRAME_NO_MOVE_FRAME. This
results in a call to nsContainerFrame::SyncFrameViewAfterReflow() that resizes
the view (line 695 of nsContainerFrame.cpp). Back up the stack, we continue in
nsScrollBoxFrame::DoLayout, which calls vm->ResizeView() again, putting the view
back to its original size.

The bug here seems to be that the call to nsContainerFrame::FinishReflowChild()
throws away the flags passed in, and hard-codes NS_FRAME_NO_MOVE_FRAME, which
results in a view resize. If it looked at the incoming flags, it would see the
NS_FRAME_NO_SIZE_VIEW flag and not resize the view.

Comment 1

[Simon Fraser [no longer active]]

Attachment: Proposed patch to nsBoxToBlockAdapter.cpp

Comment 2

[Simon Fraser [no longer active]]

I verified that with this patch, we no longer redraw the entire contents of the
textarea for each keypress.

See also bug 151682.

Comment 3

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

Comment on attachment 109682 [details] [diff] [review]
Proposed patch to nsBoxToBlockAdapter.cpp

r+sr=roc+moz

This makes total sense.

Comment 4

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

Note that this could expose bugs where reflowing doesn't repaint everything it
needs to. But we should just fix those bugs.

Also note that we'll still see full repaints when the scrolled area grows (e.g.
when a new line is added to the textarea). To fix that, we should make the
(hidden) aRepaintExposedAreaOnly parameter to ResizeView() always be PR_TRUE and
fix any "insufficient repaint" bugs that arise.

Comment 5

[Simon Fraser [no longer active]]

We still do view resizing with this patch, as soon as there is enough content to
show scrollbars (even when typing doesn't change the contents size).

Comment 6

[Simon Fraser [no longer active]]

In the case where the textarea has enough content to scroll, the view resizing
is happening via the call to PlaceFrameView() in nsBlockFrame::ReflowDirtyLines.
PlaceFrameView() ignores any reflow flags, and, anyway, the
NS_FRAME_NO_SIZE_VIEW flag doesn't make it into the nsHTMLReflowMetrics. So more
work is required to fix this case.

Comment 7

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

Are you going to check this in, or am I? I guess I'll check it in if you don't
get to it first.

Comment 8

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

Fix checked in. Leaving open to handle the remaining case.

Comment 9

[Simon Fraser [no longer active]]

Thanks. I was waiting to get a fix for the other case, but having it in is OK.

Comment 10

[Boris Zbarsky [:bzbarsky]]

So is this still an issue?

Comment 11

[Mounir Lamouri (:mounir)]

Is this still an issue?