86852 - Text not always repainted after deleting lines

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[[e-mail]]

Using Build ID: 2001061304

Steps to reproduce problem:
1. Create or open a long document (3 pages of text works best).
2. Scroll to the bottom of the document.
3. Place the caret in the middle of the window.
4. Hold down BACKSPACE (or DELETE).

Expected results: As lines are deleted the beginning of the document appears.

Actual results: Only the visible portion of the document scrolls.

Additional information:
If you use BACKSPACE you will see two lines do repaint.
When the vertical scrollbar disappears then the document repaints.
If you obscure and reveal the window then the document repaints.

Comment 1

[rubydoo123]

this sounds more like a rendering issue, assigning to mcclusky and cc myself

Comment 2

[Kathleen :Brade]

*** Bug 86969 has been marked as a duplicate of this bug. ***

Comment 3

[Koike Kazuhiko]

*** Bug 87310 has been marked as a duplicate of this bug. ***

Comment 4

[Peter Lairo]

Could we please rename the summary to something more meaningful? This is a
serious issue and might get more exposure if people actually knew what this bug
is talking about. I suggest:

"Deleting large text portions (multiple lines) in msg compose causes remaining
text display to become messed up." (or maybe at least add it to the whiteboard).

SPAM: neil - how about some keywords? e.g. *mozilla0.9.3, correctness, 4xp,
regression*

Comment 5

[[e-mail]]

This applies to editor and msg compose, both plain and html text.
However HTML textareas are unaffected.

Comment 6

[Kevin McCluskey (gone)]

Looks like its not invalidating the lines below the area that is deleting
properly. My guess is that the block frame is not invalidating its container
properly when the line is deleted.


Marking Mozilla 0.9.3, nsbranch.

Comment 7

[Marc Attinasi]

CC'ing waterson ('cause I know he loves these). Haven't we seen this problem
elsewhere - dup?

Comment 8

[Chris Waterson]

Mmm, could be bug 73348.

Comment 9

[Ninoschka Baca]

I was able to reproduce the problem on WinMe, Win98 and Mac 9.04
using an email that Clayton Lewis sent this morning titled "PDT Wednesday 26 June".

Comment 10

[rubydoo123]

I worked through some of the issues that Ninoschka mentioned. There are repaint
issues when deleting in mail compose, bullet item content stays visible until
redraw is forced, blocks of text 'disappear' when selecting Edit|Delete, again
when a redraw is forced it reappears.

Comment 11

[Koike Kazuhiko]

*** Bug 88833 has been marked as a duplicate of this bug. ***

Comment 12

[Koike Kazuhiko]

*** Bug 88285 has been marked as a duplicate of this bug. ***

Comment 13

[Jacek Piskozub]

Changing severity to major per one of the dup bugs.

Comment 14

[Marc Attinasi]

I think I have a fix for this... taking.

Comment 15

[Marc Attinasi]

Attachment: Patch to fix the way resized views are invalidated

Comment 16

[Marc Attinasi]

I've attached a patch that fixes an error in an optimization that kmcclusk added
some time ago. When a view is resized, a flag is passed telling it to 'repaint
exposed area only', but that flag was getting set to FALSE only if the width was
constant. In fact, the height needs to be checked too, otherwise we skip
invalidating the parts of the view above the affected frame when stuff is deleted.

I tested this for impact on typing performance and I could not detect any. Since
a change in the height of the frame will cause a refresh of the entire view,
however, there should be at least a small impact when more lines are added
(pressing the CR). I couldn't really tell though. It is possible that the test
for the change in the frame size could be further optimized to look for a change
in the width and a DECREASE in the height, but I'm not convinced that this is
safe yet - continuing to test.

Comment 17

[Marc Attinasi]

Performance is fine with the patch on my Mac and Win boxes, and the repainting
problems are gone. Even editing very large files (I tried a source file like
nsBlockFrame.cpp), I cannot detect a performance difference with or without the
change. Also, restricting the change to the case where the height of the frame
decreases feels identical performacne-wise, and likewise fixes the bug. The dups
with recipes are fixed too.

Requesting reviews for the patch...

Comment 18

[Chris Waterson]

[s]r=waterson

Comment 19

[Alexandru Savulov]

r=Alex Savulov

Comment 20

[Marc Attinasi]

Fix checked into trunk. nsContainerFrame.cpp v1.110

Comment 21

[Marc Attinasi]

marking VTRUNK, hoping I understand the kwd correctly. Basically, I need this
verified on the trunk so I can get clearance to land it on the trunk. Thanks.

Comment 22

[sujay]

vtrunk is a QA keyword which is a reminder for QA personnel to verify this
bug on the trunk. 

Because QA is testing on the branch right now, we need a reminder 
to test certain/most recently fixed bugs on the trunk also.

Comment 23

[Marc Attinasi]

Marking FIXED (fixed on the trunk only).

Comment 24

[Peter Lairo]

SPAM:
what is the difference between *trunk* and *branch*? Is one of them the
nightlies and the other -what-?

Comment 25

[sujay]

okay, this is working fine on the trunk..I have verified...you can check into
the branch....then I will re-verify this puppy on the branch.

adding testpage to URL field.

Comment 26

[sujay]

removing vtrunk keyword, already verified on trunk.

Comment 27

[sujay]

Marc, any idea when this will get into branch ? thanks.
I presume you'll signal me when you do this....thanks!

Comment 28

[Marc Attinasi]

This will get into the branch when it is marked PDT+ (by the PDT) or when the
branch opens up for nsBranch+ checkins (again, decided by PDT). If you think
this should be in for the next release, then you might want to lobby the PDT to
allow it in... And yes, I will let you know as soon as I land it on the branch :) 

I'm re-opening this since it is not on the branch yet (that seems to be the
generally accepted thang these days).

Comment 29

[Marc Attinasi]

This patch caused a regression in bug 90503. The fix that restricts the
full-view invalidation to cases where the height decreases is better as it fixes
this bug and the regression in bug 90503. I'll be getting that change in to the
trunk, and when the branch is ready (if ever) that is what should go there too.

Comment 30

[Marc Attinasi]

Attachment: New Patch: restrict full-view invalidation unless width is changed or height decreases (fixes regression in bug 90503)

Comment 31

[Chris Waterson]

[s]r=waterson

Comment 32

[karnaze (gone)]

r=karnaze

Comment 33

[Marc Attinasi]

Fix committed to trunk. nsContainerFrame.cpp r1.113

Comment 34

[Marc Attinasi]

Marking FIXED - we'll still need to push this to the branch at some date, but
the nsBranch+ kwd should take care of that :)

Comment 35

[James Baxter]

Verified in 9-06 build

Comment 36

[Dimitrios]

on build 2001090603, Win98:
It was really fixed but I'm seeing the problem in recent builds (when I mark a
block of text in the middle of the relatively long text body). At least when
replying. I can provide screenshot if needed. Please re-open the bug.

Comment 37

[Dimitrios]

Correction: in my previous comment, read "when I mark and delete a block of
text". Sorry.