If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Text not always repainted after deleting lines

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
Editor
P2
major
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: Marc Attinasi)

Tracking

({regression})

Trunk
mozilla0.9.3
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [QAHP] nsBranch+, URL)

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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

17 years ago
this sounds more like a rendering issue, assigning to mcclusky and cc myself
Assignee: beppe → kmcclusk

Comment 2

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

Comment 3

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

Comment 4

17 years ago
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*
(Reporter)

Comment 5

17 years ago
This applies to editor and msg compose, both plain and html text.
However HTML textareas are unaffected.
Keywords: correctness, regression
Summary: Most of area scrolled down into view by deletion not painted. → Text not always repainted after deleting lines
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.
Status: NEW → ASSIGNED
Keywords: nsBranch
Target Milestone: --- → mozilla0.9.3
(Assignee)

Comment 7

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

Comment 8

17 years ago
Mmm, could be bug 73348.

Comment 9

17 years ago
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

17 years ago
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.

Updated

17 years ago
Whiteboard: [QAHP]

Comment 11

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

Comment 12

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

Comment 13

16 years ago
Changing severity to major per one of the dup bugs.
Severity: normal → major
(Assignee)

Comment 14

16 years ago
I think I have a fix for this... taking.
Assignee: kmcclusk → attinasi
Status: ASSIGNED → NEW
(Assignee)

Comment 15

16 years ago
Created attachment 40967 [details] [diff] [review]
Patch to fix the way resized views are invalidated
(Assignee)

Comment 16

16 years ago
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.
Status: NEW → ASSIGNED
(Assignee)

Comment 17

16 years ago
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...
OS: Windows 95 → All
Priority: -- → P2
Hardware: PC → All

Comment 18

16 years ago
[s]r=waterson

Comment 19

16 years ago
r=Alex Savulov
(Assignee)

Comment 20

16 years ago
Fix checked into trunk. nsContainerFrame.cpp v1.110
(Assignee)

Comment 21

16 years ago
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.
Keywords: vtrunk

Comment 22

16 years ago
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.
(Assignee)

Comment 23

16 years ago
Marking FIXED (fixed on the trunk only).
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 24

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

Comment 25

16 years ago
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

16 years ago
removing vtrunk keyword, already verified on trunk.
Keywords: vtrunk

Updated

16 years ago
Whiteboard: [QAHP] → [QAHP] nsBranch+

Comment 27

16 years ago
Marc, any idea when this will get into branch ? thanks.
I presume you'll signal me when you do this....thanks!
(Assignee)

Comment 28

16 years ago
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).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 29

16 years ago
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.
(Assignee)

Comment 30

16 years ago
Created attachment 42432 [details] [diff] [review]
New Patch: restrict full-view invalidation unless width is changed or height decreases (fixes regression in bug 90503)
(Assignee)

Updated

16 years ago
Blocks: 90503

Comment 31

16 years ago
[s]r=waterson

Comment 32

16 years ago
r=karnaze
(Assignee)

Comment 33

16 years ago
Fix committed to trunk. nsContainerFrame.cpp r1.113
Status: REOPENED → ASSIGNED
(Assignee)

Comment 34

16 years ago
Marking FIXED - we'll still need to push this to the branch at some date, but
the nsBranch+ kwd should take care of that :)
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 35

16 years ago
Verified in 9-06 build
Status: RESOLVED → VERIFIED

Comment 36

16 years ago
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

16 years ago
Correction: in my previous comment, read "when I mark and delete a block of
text". Sorry.
You need to log in before you can comment on or make changes to this bug.